[Framework] Delete entities which didn't pass the mapping selector in register_raw #490

omby8888 · 2024-04-08T12:34:13Z

Description

What - Delete entities which didn't pass the mapping selector in register_raw

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)

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.

…ded a ownership check on real time deletion

…e-entity-on-real-time-events # Conflicts: # integrations/dynatrace/config.yaml # port_ocean/clients/port/mixins/entities.py

Tankilevitch

I would prefer that you organize the methods you use, and separate them so its more readable other than trying to re-use existing ones. When using the unregister_raw and register_raw there is no meaning for passing it as diff IIRC

Tankilevitch · 2024-04-08T12:35:37Z

docs/framework-guides/docs/framework/features/live-events.md

@@ -58,7 +58,7 @@ For example, if you configure the integration to listen to the `/webhook` endpoi

 :::

-Here is an example definition that exposes a `/integrations/webhook` route the integration will listen to:
+Here is an example definition that exposes a `/integration/webhook` route the integration will listen to:


Tankilevitch · 2024-04-16T10:16:25Z

port_ocean/clients/port/mixins/entities.py

-        query = {
-            "combinator": "and",
+    async def search_entities(
+        self, user_agent_type: UserAgentType, query: dict[Any, Any] | None = None
+    ) -> list[Entity]:
+        default_query = {
+            "combinator": "or",


seems like the default search entities has changed from and to or is that on purpose, this has a huge performance impact on the query behavior and might result with different results

Tankilevitch · 2024-04-16T10:18:48Z

port_ocean/core/handlers/entity_processor/jq_entity_processor.py

        self,
        data: dict[str, Any],
        raw_entity_mappings: dict[str, Any],
        selector_query: str,
+        ignore_selector: bool = False,


there is inconsistency with the parameter across all the functions that pass it through.

Tankilevitch · 2024-04-16T10:45:05Z

port_ocean/core/handlers/entity_processor/jq_entity_processor.py

+        if ignore_selector or should_run:
+            mapped_entity = await self._search_as_object(data, raw_entity_mappings)
+            mapped_entity[DOES_ENTITY_PASSED_SELECTOR_KEY] = should_run
+            return mapped_entity


I am not a fan at all of passing flags through unrelated objects, you can change the data structure of how you return that data, we can discuss it offline, but injecting into an object which is user related shouldn't be done, and is very hard to debug.

…e-entity-on-real-time-events

Tankilevitch · 2024-04-16T14:26:49Z

port_ocean/core/handlers/entity_processor/jq_entity_processor.py

+        passed = []
+        failed = []


Suggested change

passed = []

failed = []

passed_entities= []

failed_entities = []

Tankilevitch · 2024-04-16T14:27:38Z

port_ocean/core/handlers/entity_processor/jq_entity_processor.py

                )
            )
-            for data in raw_data
+            for data in raw_results
        ]
        entities = await asyncio.gather(*entities_tasks)


Suggested change

entities = await asyncio.gather(*entities_tasks)

entities_results = await asyncio.gather(*entities_tasks)

Tankilevitch · 2024-04-16T14:32:04Z

port_ocean/core/handlers/entity_processor/jq_entity_processor.py

+        for flatten in entities:
+            for entity_data, did_entity_pass_selector in flatten:


Suggested change

for flatten in entities:

for entity_data, did_entity_pass_selector in flatten:

for entity_result in entities_results:

for entity, did_entity_pass_selector in entity_result:

btw I don't think this unpacking of tuple will work as flatten isn't an iterator.
I think entity_result[0] entity_result[1] will suit better

i'll make sure but flatten is a list of tuples so it should work

yeah it works

Tankilevitch · 2024-04-17T12:34:00Z

port_ocean/core/integrations/mixins/sync_raw.py

+            if not is_owner:
+                logger.info(
+                    f"Entity {entity_to_delete.identifier} hasn't been created by this integration, "
+                    f"skipping deletion..."
+                )
+                continue
+            await self.entities_state_applier.delete(
+                objects_diff[0]["failed"], user_agent_type


this log might be confusing, as it is possible that the entity doesn't exists in port which in that case will return is_owner=False as well.

Tankilevitch · 2024-04-17T14:34:27Z

port_ocean/core/handlers/entity_processor/jq_entity_processor.py

-            "before": entities_before,
-            "after": entities_after,
-        }
+        entities_results = await asyncio.gather(*entities_tasks)


Suggested change

entities_results = await asyncio.gather(*entities_tasks)

calculate_entities_results = await asyncio.gather(*calculate_entities_tasks)

Tankilevitch · 2024-04-17T14:34:50Z

port_ocean/core/handlers/entity_processor/jq_entity_processor.py

+        for entity_result in entities_results:
+            for entity, did_entity_pass_selector in entity_result:


Suggested change

for entity_result in entities_results:

for entity, did_entity_pass_selector in entity_result:

for entities_results in calculate_entities_results:

for entity, did_entity_pass_selector in entities_results:

Tankilevitch · 2024-04-17T14:57:55Z

port_ocean/core/integrations/mixins/sync_raw.py

+                    f"Entity {entity_to_delete.identifier} either hasn't been created by this integration "
+                    f"or it doesn't exist, skipping deletion..."


Skipping deletion of entity {entity_to_delete.identifier}, couldn't find entity that is related to integration.

…e-entity-on-real-time-events

Tankilevitch

LGTM

yairsimantov20 · 2024-04-14T08:12:11Z

port_ocean/core/ocean_types.py

@@ -3,16 +3,24 @@
 from port_ocean.core.models import Entity


+RawEntity = dict[Any, Any]


use raw item instead

yairsimantov20 · 2024-04-14T08:32:39Z

port_ocean/clients/port/mixins/entities.py

+        if query is None:
+            query = default_query
+        elif query.get("rules"):
+            query["rules"].append(default_query)
+


why not splitting this function?

Delete entities which didn't pass the mapping selector in register_raw

0cdf0cf

github-actions bot added framework A change in the ocean framework files size/S labels Apr 8, 2024

added a parameter to decide whether to parse all raw entities and add…

1673875

…ded a ownership check on real time deletion

github-actions bot added size/L integration A change in integration files and removed size/S labels Apr 9, 2024

Reverted oopsie change

0f22d8c

github-actions bot removed the integration A change in integration files label Apr 9, 2024

omby8888 added 3 commits April 9, 2024 17:43

change var names

8a7997d

fixed linting

a615976

fixed linting

de84937

github-actions bot added the integration A change in integration files label Apr 10, 2024

omby8888 added 4 commits April 10, 2024 15:16

reverted oopsie change

f65e219

Merge remote-tracking branch 'origin/main' into PORT-7494-Ocean-Delet…

3333a15

…e-entity-on-real-time-events # Conflicts: # integrations/dynatrace/config.yaml # port_ocean/clients/port/mixins/entities.py

pulled main

e636b9b

pulled main

ba566f5

Tankilevitch requested changes Apr 16, 2024

View reviewed changes

omby8888 added 3 commits April 16, 2024 15:36

Merge remote-tracking branch 'origin/main' into PORT-7494-Ocean-Delet…

86fec69

…e-entity-on-real-time-events

cr fixes

17a0208

cr fixes

fe213d0

Tankilevitch requested changes Apr 16, 2024

View reviewed changes

cr fixes

b360d6b

Tankilevitch reviewed Apr 17, 2024

View reviewed changes

cr fixes

a13ee51

Tankilevitch reviewed Apr 17, 2024

View reviewed changes

omby8888 added 3 commits April 17, 2024 18:12

cr fixes

f693f7d

Merge remote-tracking branch 'origin/main' into PORT-7494-Ocean-Delet…

6010911

…e-entity-on-real-time-events

bump version

3d04469

omby8888 changed the title ~~Delete entities which didn't pass the mapping selector in register_raw~~ [Framework] Delete entities which didn't pass the mapping selector in register_raw Apr 17, 2024

Tankilevitch approved these changes Apr 17, 2024

View reviewed changes

omby8888 merged commit f3ce269 into main Apr 17, 2024
5 checks passed

omby8888 deleted the PORT-7494-Ocean-Delete-entity-on-real-time-events branch April 17, 2024 15:46

yairsimantov20 reviewed Apr 17, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Framework] Delete entities which didn't pass the mapping selector in register_raw #490

[Framework] Delete entities which didn't pass the mapping selector in register_raw #490

omby8888 commented Apr 8, 2024

Tankilevitch left a comment

Tankilevitch Apr 8, 2024

Tankilevitch Apr 16, 2024

omby8888 Apr 16, 2024

Tankilevitch Apr 16, 2024

Tankilevitch Apr 16, 2024

Tankilevitch Apr 16, 2024

Tankilevitch Apr 16, 2024

Tankilevitch Apr 16, 2024

Tankilevitch Apr 16, 2024

omby8888 Apr 16, 2024 •

edited

omby8888 Apr 16, 2024

Tankilevitch Apr 17, 2024

Tankilevitch Apr 17, 2024

Tankilevitch Apr 17, 2024

Tankilevitch Apr 17, 2024

Tankilevitch left a comment

yairsimantov20 Apr 14, 2024

yairsimantov20 Apr 14, 2024

omby8888 Apr 18, 2024

	entities = await asyncio.gather(*entities_tasks)
	entities_results = await asyncio.gather(*entities_tasks)

		for flatten in entities:
		for entity_data, did_entity_pass_selector in flatten:

	entities_results = await asyncio.gather(*entities_tasks)
	calculate_entities_results = await asyncio.gather(*calculate_entities_tasks)

		for entity_result in entities_results:
		for entity, did_entity_pass_selector in entity_result:

		f"Entity {entity_to_delete.identifier} either hasn't been created by this integration "
		f"or it doesn't exist, skipping deletion..."

		@@ -3,16 +3,24 @@
		from port_ocean.core.models import Entity


		RawEntity = dict[Any, Any]

[Framework] Delete entities which didn't pass the mapping selector in register_raw #490

[Framework] Delete entities which didn't pass the mapping selector in register_raw #490

Conversation

omby8888 commented Apr 8, 2024

Description

Type of change

Screenshots

API Documentation

Tankilevitch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omby8888 Apr 16, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tankilevitch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omby8888 Apr 16, 2024 •

edited