Skip to content

Commit

Permalink
code review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
anshbansal committed Jan 3, 2025
1 parent 8872b09 commit 61c5b14
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,9 @@ def _scroll_execution_requests(
break
if self.report.ergc_read_errors >= self.config.max_read_errors:
self.report.failure(
f"ergc({self.instance_id}): too many read errors, aborting."
title="Too many read errors, aborting",
message="Too many read errors, aborting",
context=str(self.instance_id),
)
break
try:
Expand All @@ -159,8 +161,8 @@ def _scroll_execution_requests(
params["scrollId"] = document["scrollId"]
except Exception as e:
self.report.failure(

Check warning on line 163 in metadata-ingestion/src/datahub/ingestion/source/gc/execution_request_cleanup.py

View check run for this annotation

Codecov / codecov/patch

metadata-ingestion/src/datahub/ingestion/source/gc/execution_request_cleanup.py#L163

Added line #L163 was not covered by tests
title="failed to fetch next batch of execution requests",
message="failed to fetch next batch of execution requests",
title="Failed to fetch next batch of execution requests",
message="Failed to fetch next batch of execution requests",
context=str(self.instance_id),
exc=e,
)
Expand Down Expand Up @@ -235,8 +237,8 @@ def _delete_entry(self, entry: CleanupRecord) -> None:
except Exception as e:
self.report.ergc_delete_errors += 1
self.report.failure(

Check warning on line 239 in metadata-ingestion/src/datahub/ingestion/source/gc/execution_request_cleanup.py

View check run for this annotation

Codecov / codecov/patch

metadata-ingestion/src/datahub/ingestion/source/gc/execution_request_cleanup.py#L239

Added line #L239 was not covered by tests
title="failed to delete ExecutionRequest",
message="failed to delete ExecutionRequest",
title="Failed to delete ExecutionRequest",
message="Failed to delete ExecutionRequest",
context=str(self.instance_id),
exc=e,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ class SoftDeletedEntitiesReport(SourceReport):
sample_hard_deleted_aspects_by_type: TopKDict[str, LossyList[str]] = field(
default_factory=TopKDict
)
runtime_limit_reached: bool = False
deletion_limit_reached: bool = False

Check warning on line 109 in metadata-ingestion/src/datahub/ingestion/source/gc/soft_deleted_entity_cleanup.py

View check run for this annotation

Codecov / codecov/patch

metadata-ingestion/src/datahub/ingestion/source/gc/soft_deleted_entity_cleanup.py#L108-L109

Added lines #L108 - L109 were not covered by tests


class SoftDeletedEntitiesCleanup:
Expand Down Expand Up @@ -207,8 +209,8 @@ def _process_futures(self, futures: Dict[Future, str]) -> Dict[Future, str]:
if future.exception():
self.report.failure(
title="Failed to delete entity",
message=futures[future],
context=str(futures[future]),
message="Failed to delete entity",
context=futures[future],
exc=future.exception(),
)
self.report.num_soft_deleted_entity_processed += 1
Expand Down Expand Up @@ -280,9 +282,8 @@ def _times_up(self) -> bool:
self.config.runtime_limit_seconds
and time.time() - self.start_time > self.config.runtime_limit_seconds
):
logger.info(
f"Runtime limit of {self.config.runtime_limit_seconds} seconds reached"
)
with self._report_lock:
self.report.runtime_limit_reached = True
return True
return False

Check warning on line 288 in metadata-ingestion/src/datahub/ingestion/source/gc/soft_deleted_entity_cleanup.py

View check run for this annotation

Codecov / codecov/patch

metadata-ingestion/src/datahub/ingestion/source/gc/soft_deleted_entity_cleanup.py#L285-L288

Added lines #L285 - L288 were not covered by tests

Expand All @@ -291,9 +292,8 @@ def _deletion_limit_reached(self) -> bool:
self.config.limit_entities_delete
and self.report.num_hard_deleted > self.config.limit_entities_delete
):
logger.info(
f"Limit of {self.config.limit_entities_delete} entities reached"
)
with self._report_lock:
self.report.deletion_limit_reached = True
return True
return False

Check warning on line 298 in metadata-ingestion/src/datahub/ingestion/source/gc/soft_deleted_entity_cleanup.py

View check run for this annotation

Codecov / codecov/patch

metadata-ingestion/src/datahub/ingestion/source/gc/soft_deleted_entity_cleanup.py#L295-L298

Added lines #L295 - L298 were not covered by tests

Expand Down

0 comments on commit 61c5b14

Please sign in to comment.