sadpandajoe commented on code in PR #45029:
URL: https://github.com/apache/superset/pull/45029#discussion_r4227587628
##########
superset/tasks/version_history_retention.py:
##########
@@ -513,20 +661,32 @@ def _prune_old_versions_impl(retention_days: int) ->
dict[str, Any]:
"pruned_child_shadows": 0,
"pruned_m2m_shadows": 0,
}
- total_retried = 0
- after_id = 0
+ total_retried: int = 0
+ after_id: int = 0
+ pass_stats: dict[str, Any]
+ retries: int
while True:
- pass_stats, retries = _run_pass_with_retry(cutoff, tables, after_id)
+ remaining_budget: int | None = (
+ max_per_run - totals["pruned_transactions"]
+ if max_per_run is not None
+ else None
+ )
+ pass_stats, retries = _run_pass_with_retry(
+ cutoff, tables, after_id, max_prune=remaining_budget
+ )
total_retried += retries
for key in totals:
totals[key] += pass_stats.get(key, 0)
+ if max_per_run is not None and totals["pruned_transactions"] >=
max_per_run:
Review Comment:
With this early exit a second run is no longer a no-op: with 2,000 eligible
transactions and the default cap, two back-to-back runs each prune 1,000, so
the module docstring's "Idempotent: a second run prunes nothing" (line 43) is
now wrong. `deletion_retention.py` has the same "a re-run with the same clock
and data removes nothing" claim in its docstring (line 25), which the per-run
cap also breaks. An operator who re-runs the task to double-check would delete
another batch. Should both docstrings describe capped runs as draining the
backlog over successive runs instead?
--
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]
