Skip to content
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
merged 10 commits into from
Aug 28, 2024

Conversation

omby8888
Copy link
Contributor

@omby8888 omby8888 commented Aug 27, 2024

Description

What - Refer to entity key in upsert entity response, if the response is not 200 then turn raw search relations into None to not harm the deletion proccess

Why - Response from upsert entity api isn't processed right and causes ocean core to use the raw entity that contains dict typed relations instead of string which is the relation's entity identifier

How - refer to the entity key in a response that looks like {ok: true, entity: {...}}, if the response is not 200 turn the dict typed relations into None

Type of change

Please leave one option from the following and delete the rest:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • New Integration (non-breaking change which adds a new integration)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Non-breaking change (fix of existing functionality that will not change current behavior)
  • Documentation (added/updated documentation)

All tests should be run against the port production environment(using a testing org).

Core testing checklist

  • Integration able to create all default resources from scratch
  • Resync finishes successfully
  • Resync able to create entities
  • Resync able to update entities
  • Resync able to detect and delete entities
  • Scheduled resync able to abort existing resync and start a new one
  • Tested with at least 2 integrations from scratch
  • Tested with Kafka and Polling event listeners

Integration testing checklist

  • Integration able to create all default resources from scratch
  • Resync able to create entities
  • Resync able to update entities
  • Resync able to detect and delete entities
  • Resync finishes successfully
  • If new resource kind is added or updated in the integration, add example raw data, mapping and expected result to the examples folder in the integration directory.
  • If resource kind is updated, run the integration with the example data and check if the expected result is achieved
  • If new resource kind is added or updated, validate that live-events for that resource are working as expected
  • Docs PR link here

Preflight checklist

  • Handled rate limiting
  • Handled pagination
  • Implemented the code in async
  • Support Multi account

Screenshots

Include screenshots from your environment showing how the resources of the integration will look.

API Documentation

Provide links to the API documentation used for this integration.

@omby8888 omby8888 requested a review from a team as a code owner August 27, 2024 14:34
CHANGELOG.md Outdated Show resolved Hide resolved
Entity.parse_obj(result["entity"]) if result.get("entity") else entity
)

result_entity.relations = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment to explain it

omby8888 and others added 3 commits August 27, 2024 17:59
Co-authored-by: talsabagport <tal@getport.io>
…rror-unhashable-dict' into PORT-10064-Bug-search-relation-error-unhashable-dict
Comment on lines +61 to +62
result_entity = (
Entity.parse_obj(result["entity"]) if result.get("entity") else entity
Copy link
Contributor

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.

Copy link
Contributor Author

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

@Tankilevitch Tankilevitch changed the title [Framework] Port 10064 bug search relation error unhashable dict [Framework] Fix bug search relation error unhashable dict Aug 27, 2024
Copy link
Contributor

@Tankilevitch Tankilevitch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@omby8888 omby8888 merged commit 9a5dbb7 into main Aug 28, 2024
12 checks passed
@omby8888 omby8888 deleted the PORT-10064-Bug-search-relation-error-unhashable-dict branch August 28, 2024 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants