Ensure that deleted nodes are removed from the schema_branch #5183
+159
−68
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
group/backend
CodSpeed Performance ReportMerging #5183 will not alter performanceComparing Summary
|
ogenstad
force-pushed
the
pog-schema-sync-deleted-nodes-IFC-887
branch
from
a579bf6 to
bd482ce
Compare
ajtmccarty
reviewed
Dec 10, 2024
| generics=[ | ||
| key for key, value in other.generics.items() if key not in self.generics or self.generics[key] != value | ||
| ], | ||
| removed_generics=[key for key in self.generics if key not in other.generics], | ||
| ) |
There was a problem hiding this comment.
it might be cleaner if these used sets instead of lists for this logic, but I understand if it is out of scope for this small change
There was a problem hiding this comment.
You mean all of the objects in SchemaBranchDiff? If so perhaps we can do that once it's brought back into develop?
ogenstad
force-pushed
the
pog-schema-sync-deleted-nodes-IFC-887
branch
from
a446ed5 to
74e0e16
Compare
ogenstad
force-pushed
the
pog-schema-sync-deleted-nodes-IFC-887
branch
8 times, most recently
from
47d71d3 to
b07cc0b
Compare
ogenstad
force-pushed
the
pog-schema-sync-deleted-nodes-IFC-887
branch
from
b07cc0b to
3164949
Compare
dgarros
approved these changes
Dec 13, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The root cause here is that when we delete a nodeschema it will be removed from the current SchemaBranch object on the API worker that handled the request. On other workers in the system the old deleted nodes are still cached in the SchemaBranch within the registry and never cleared out (until the worker is restarted).
Within infrahub/core/merge.py I also completely removed the get_schema_diff() method that appeared to be dead code that wasn't reached by any path.
Initially I added all modified nodes to SchemaBranchDiff (the current compare function only looks add added nodes from one side), however this was a bit problematic as we couldn't see which ones were added and as such it was complicated to remove nodes as it wasn't clear about what action to take.
This PR modifies the SchemaBranchDiff object to specifically include a section for specific actions on nodes and generics, i.e. added/changed/removed. This changed forced some additional changes around the logic of this object.
I've added a test to validate that this works. However for now that pipeline is disabled due to problems related to starting the actual containers. The tests work locally though (however we have to switch back and forth between which environment variables are active when doing so), as a follow up to enable that pipeline I've opened #5205.
Fixes #4836