sadpandajoe commented on code in PR #45029:
URL: https://github.com/apache/superset/pull/45029#discussion_r4225520472
##########
superset/tasks/deletion_retention.py:
##########
@@ -133,71 +143,178 @@ def _report_model_counts(outcome: str, counts: dict[str,
int]) -> None:
)
-@dataclass
-class _PassTotals:
- """What one pass over the soft-delete roots produced."""
+def _count_eligible(model: type[SoftDeleteMixin], cutoff: datetime) -> int:
+ """Count aged roots, including roots that a later cascade may block."""
+ table: sa.Table = _model_table(model)
+ with skip_visibility_filter(db.session, model):
+ return int(
+ db.session.scalar(
+ sa.select(sa.func.count())
+ .select_from(table)
+ .where(table.c.deleted_at.is_not(None), table.c.deleted_at <
cutoff)
Review Comment:
`remaining_eligible` and `estimated_capped_runs` are built on this query,
but every test patches `_count_eligible` (`side_effect=[0, 3, 0, 2]` etc.) and
the integration test only exercises the version-history side, so the predicate
itself never runs against a database. If `table.c.deleted_at < cutoff` were
dropped or flipped, recently archived roots would be reported as eligible
backlog and operators would size the cap against a number that is too large,
with every test still green. Could a DB-backed case seed roots before, exactly
at and after the cutoff plus a live (not soft-deleted) root, and assert the
count is only the before-cutoff one?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
