mikebridge commented on code in PR #45029:
URL: https://github.com/apache/superset/pull/45029#discussion_r4226814056
##########
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:
Good catch: every caller patched `_count_eligible`, so its predicate never
ran.
[d4c8c32469](https://github.com/apache/superset/commit/d4c8c3246924a9127d6482846f3582c39f0c3843)
adds a SQLite-backed test on the real `slices` table. It seeds two roots
archived before the cutoff, one exactly at the cutoff, one after it and one
live root, then asserts the count is 2. It also asserts the purge scan selects
the same two IDs, so the test checks that the reported backlog agrees with the
rows the purge scans.
- Dropping the cutoff condition fails the test (4 ≠ 2).
- Changing `<` to `<=` fails it (3 ≠ 2).
- Flipping the comparison fails it (1 ≠ 2).
All 405 focused retention tests and the branch-file pre-commit checks pass.
--
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]
