-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Framework] Fix bug search relation error unhashable dict #956
Merged
omby8888
merged 10 commits into
main
from
PORT-10064-Bug-search-relation-error-unhashable-dict
Aug 28, 2024
Merged
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
97f8116
handle upsert failure
omby8888 9a9a870
handle upsert failure
omby8888 5e4cbb7
handle upsert failure
omby8888 5f358c1
Update CHANGELOG.md
omby8888 7cdb0b3
handle upsert failure
omby8888 96ffa2d
Merge remote-tracking branch 'origin/PORT-10064-Bug-search-relation-e…
omby8888 a33d9be
cr fixes
omby8888 9541c49
cr fixes
omby8888 bd37dae
Merge remote-tracking branch 'origin/main' into PORT-10064-Bug-search…
omby8888 1f48713
upgraded version
omby8888 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,11 +58,19 @@ async def upsert_entity( | |
) | ||
handle_status_code(response, should_raise) | ||
result = response.json() | ||
result_entity = Entity.parse_obj(result) | ||
# Set the results of the search relation and identifier to the entity | ||
entity.identifier = result_entity.identifier or entity.identifier | ||
entity.relations = result_entity.relations or entity.relations | ||
return entity | ||
result_entity = ( | ||
Entity.parse_obj(result["entity"]) if result.get("entity") else entity | ||
) | ||
|
||
# Turning dict typed relations (raw search relations) is required | ||
# for us to be able to successfully calculate the participation related entities | ||
# and ignore the ones that don't as they weren't upserted | ||
result_entity.relations = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a comment to explain it |
||
key: None if isinstance(relation, dict) else relation | ||
for key, relation in result_entity.relations.items() | ||
} | ||
|
||
return result_entity | ||
|
||
async def batch_upsert_entities( | ||
self, | ||
|
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
returning all the result entity means that we return the full entity that is inside port, this means that if we just ingested the relations of an entity but it has 20 more properties it will be returned here and passed forward to the resync phase. This can be problematic from a memory point of view and that is why we only took the identifier and the relations from the result entity and not the full entity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need the blueprint as well to delete the identifier of a specific blueprint only as there may be entities with same identifier but different blueprints