sadpandajoe commented on code in PR #45029:
URL: https://github.com/apache/superset/pull/45029#discussion_r4222006009
##########
tests/unit_tests/tasks/test_deletion_retention.py:
##########
@@ -694,4 +1099,273 @@ def
test_standalone_window_bounds_reach_safe_purge_cutoff(
assert result == {"skipped": 1}
purge.assert_not_called()
else:
- purge.assert_called_once_with(Slice, now - timedelta(days=expected),
False)
+ purge.assert_called_once_with(
+ Slice, now - timedelta(days=expected), False, max_per_run=None
+ )
+
+
+def test_purge_cap_counts_committed_roots_across_batches(app_context: None) ->
None:
+ """A failed or blocked root does not consume the successful-purge
budget."""
+ from superset.commands.deletion_retention.purge_cascade import
CascadeResult
+ from superset.models.slice import Slice
+ from superset.tasks import deletion_retention as task
+
+ purged: CascadeResult = CascadeResult(
+ purged=True, entity_type="chart", entity_uuid="purged"
+ )
+ not_purged: CascadeResult = CascadeResult(
+ purged=False, entity_type="chart", entity_uuid="not-purged"
+ )
+ purge_one: MagicMock
+ with (
+ patch.object(task, "_iter_eligible_ids", return_value=[[1, 2], [3,
4]]) as scan,
+ patch.object(
+ task, "_purge_one", side_effect=[not_purged, purged, purged]
+ ) as purge_one,
+ ):
+ result: task._PurgeModelResult = task._purge_model(
+ Slice, datetime.now(), dry_run=False, max_per_run=2
+ )
+
+ assert result == task._PurgeModelResult(2, 0, 0, 0, 0)
+ assert [call.args[1] for call in purge_one.call_args_list] == [1, 2, 3]
+ scan.assert_called_once()
+
+
+def test_scheduled_purge_rejects_invalid_cap_before_work(app_config: Config)
-> None:
Review Comment:
Nothing pins that the scheduled purge forwards a positive cap: every
scheduled-entrypoint test here uses `0` (unlimited) or an invalid value, and
the positive-cap tests call `_purge_impl`/`_purge_model` directly. Hard-coding
`max_per_run=None` at the `_purge_impl(...)` call in `purge_soft_deleted` would
leave this file green while the default `1000` is silently ignored. Could you
add a scheduled-entrypoint case with `SOFT_DELETE_PURGE_MAX_PER_RUN` set to `2`
that asserts `_purge_impl` receives `max_per_run=2`?
--
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]
