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

[Core] Port 11686 Reduce cases of mass deletion #1363

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions port_ocean/core/handlers/entities_state_applier/port/applier.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ async def delete_diff(
self,
entities: EntityDiff,
user_agent_type: UserAgentType,
entity_deletion_threshold: float = 0.9,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think default here should be Null and the value will be passed

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it will be clearer to communicate it through precentage e.g. 90 rather than 0.9, wdyt?

) -> None:
diff = get_port_diff(entities["before"], entities["after"])

Expand All @@ -87,10 +88,21 @@ async def delete_diff(
kept_entities = diff.created + diff.modified

logger.info(
f"Determining entities to delete ({len(diff.deleted)}/{len(kept_entities)})"
f"Determining entities to delete ({len(diff.deleted)}/{len(kept_entities)})",
deleting_entities=len(diff.deleted),
keeping_entities=len(kept_entities),
)
Comment on lines +93 to 94
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
keeping_entities=len(kept_entities),
)
keeping_entities=len(kept_entities),
entity_deletion_threshold=entity_deletion_threshold
)


await self._safe_delete(diff.deleted, kept_entities, user_agent_type)
delete_rate = len(diff.deleted) / len(entities["before"])
Copy link
Contributor

Choose a reason for hiding this comment

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

lets use another parameter name

if delete_rate <= entity_deletion_threshold:
await self._safe_delete(diff.deleted, kept_entities, user_agent_type)
else:
logger.info(
f"Skipping deletion of entities with delete rate {delete_rate}",
delete_rate=delete_rate,
deleting_entities=len(diff.deleted),
total_entities=len(entities),
)

async def upsert(
self, entities: list[Entity], user_agent_type: UserAgentType
Expand Down
3 changes: 3 additions & 0 deletions port_ocean/core/handlers/port_app_config/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ class PortAppConfig(BaseModel):
create_missing_related_entities: bool = Field(
alias="createMissingRelatedEntities", default=True
)
entity_deletion_threshold: float = Field(
alias="entityDeletionThreshold", default=0.9
)
resources: list[ResourceConfig] = Field(default_factory=list)

def get_port_request_options(self) -> RequestOptions:
Expand Down
2 changes: 1 addition & 1 deletion port_ocean/core/integrations/mixins/sync_raw.py
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ async def sync_raw_all(
)
await self.entities_state_applier.delete_diff(
{"before": entities_at_port, "after": generated_entities},
user_agent_type,
user_agent_type, app_config.entity_deletion_threshold
)

logger.info("Resync finished successfully")
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
from unittest.mock import Mock, patch
import pytest
from port_ocean.core.handlers.entities_state_applier.port.applier import (
HttpEntitiesStateApplier,
)
from port_ocean.core.models import Entity
from port_ocean.core.ocean_types import EntityDiff
from port_ocean.clients.port.types import UserAgentType


@pytest.mark.asyncio
async def test_delete_diff_no_deleted_entities() -> None:
applier = HttpEntitiesStateApplier(Mock())
entities = EntityDiff(
before=[Entity(identifier="1", blueprint="test")],
after=[Entity(identifier="1", blueprint="test")],
)

with patch.object(applier, "_safe_delete") as mock_safe_delete:
await applier.delete_diff(entities, UserAgentType.exporter)

mock_safe_delete.assert_not_called()


@pytest.mark.asyncio
async def test_delete_diff_below_threshold() -> None:
applier = HttpEntitiesStateApplier(Mock())
entities = EntityDiff(
before=[
Entity(identifier="1", blueprint="test"),
Entity(identifier="2", blueprint="test"),
Entity(identifier="3", blueprint="test"),
],
after=[
Entity(identifier="1", blueprint="test"),
Entity(identifier="2", blueprint="test"),
],
)

with patch.object(applier, "_safe_delete") as mock_safe_delete:
await applier.delete_diff(entities, UserAgentType.exporter)

mock_safe_delete.assert_called_once()
assert len(mock_safe_delete.call_args[0][0]) == 1
assert mock_safe_delete.call_args[0][0][0].identifier == "3"


@pytest.mark.asyncio
async def test_delete_diff_above_default_threshold() -> None:
applier = HttpEntitiesStateApplier(Mock())
entities = EntityDiff(
before=[
Entity(identifier="1", blueprint="test"),
Entity(identifier="2", blueprint="test"),
Entity(identifier="3", blueprint="test"),
],
after=[],
)

with patch.object(applier, "_safe_delete") as mock_safe_delete:
await applier.delete_diff(entities, UserAgentType.exporter)

mock_safe_delete.assert_not_called()


@pytest.mark.asyncio
async def test_delete_diff_custom_threshold_above_threshold_not_deleted() -> None:
applier = HttpEntitiesStateApplier(Mock())
entities = EntityDiff(
before=[
Entity(identifier="1", blueprint="test"),
Entity(identifier="2", blueprint="test"),
],
after=[],
)

with patch.object(applier, "_safe_delete") as mock_safe_delete:
await applier.delete_diff(
entities, UserAgentType.exporter, entity_deletion_threshold=0.5
)

mock_safe_delete.assert_not_called()
Loading