Fix too many relationship error #5541
Merged
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 #5541 will not alter performanceComparing Summary
|
LucasG0
force-pushed
the
lgu-fix-prefix-too-many-rels
branch
from
eaa8f89 to
ef1adef
Compare
LucasG0
commented
Jan 21, 2025
| # having the same parent, with the parent having another extra relationship. Concurrent coroutines will try to | ||
| # add this relationship to this object, and the second one will fail. See https://github.com/opsmill/infrahub/issues/5474. | ||
| # Note it also prevents extra relationships fetching. Ideally, DataLoader would fetch all needed data only once | ||
| # and exporting node to graphql would then not mutate this object. |
There was a problem hiding this comment.
Additional thoughts regarding why ip_namespace relationships of parent prefix is not used from DataLoader in below test, based on debugging/discussion with @ajtmccarty. I am not 100% sure but I believe it makes sense:
DataLoadercorrectly loads prefix 10.0.0.0/8, but when we resolve parent of 10.0.0.0/16 (ie, 10.0.0.0/8),node_fieldsappear to be different between first call to_get_entities_with_data_loaderfor 10.0.0.0/8 and second call to resolve parent of 10.0.0.0/16 (ie, 10.0.0.0/8). Thus we do not match existingDataLoaderand cannot retrieve existing node already havingip_namespacefetched.- Now, if we had queried only 10.0.0.0/16 and 10.0.0.1/16, 10.0.0.0/8 would have been retrieved only while resolving parent of these nodes. However, graphql query does not ask for parent
ip_namespacerelationship so it's not retrieved byDataLoader, but it does ask for hfid which relies onip_namespaceand then we fetch it afterDataLoaderstep.
There was a problem hiding this comment.
Update as per @ajtmccarty slack comment, ip_namespace is probably not retrieved correctly because we resolve parent with kind BuiltinIPPrefix for which hfid does not contain ip_namespace, instead of InfraPrefix
dgarros
approved these changes
Jan 21, 2025
ajtmccarty
approved these changes
Jan 22, 2025
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.
Fixes #5474, #5475