This is an automated email from the ASF dual-hosted git repository.
aminghadersohi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 21b937b59d7 fix(retention): skip models without purge policies before
scanning (#44874)
21b937b59d7 is described below
commit 21b937b59d7c5146fdac31572c12f44bd5032b3a
Author: Mike Bridge <[email protected]>
AuthorDate: Thu Oct 1 23:09:06 2026 -0600
fix(retention): skip models without purge policies before scanning (#44874)
---
docs/docs/using-superset/recently-archived.mdx | 10 ++
superset/tasks/deletion_retention.py | 55 ++++---
tests/unit_tests/tasks/test_deletion_retention.py | 171 +++++++++++++++++++++-
3 files changed, 217 insertions(+), 19 deletions(-)
diff --git a/docs/docs/using-superset/recently-archived.mdx
b/docs/docs/using-superset/recently-archived.mdx
index becd4c8d543..e2347ac40d7 100644
--- a/docs/docs/using-superset/recently-archived.mdx
+++ b/docs/docs/using-superset/recently-archived.mdx
@@ -115,6 +115,16 @@ The `deletion_retention.purge_soft_deleted` background
task purges eligible
archived objects older than the retention window. The default Celery beat
configuration schedules it daily at 00:00 in the configured Celery timezone.
The task skips purging when `SOFT_DELETE` is off or the window is zero.
+Before scanning a model's rows, it checks for a purge policy. Unsupported
+soft-deletable models are retained without a scan or a per-row purge attempt.
+Both live and dry runs report `unsupported_models`, a map from table name to
+`1` for each skipped model (not a row count), and log one warning per model.
+Each skip increments `deletion_retention.unsupported_models.<table>` once per
+run. Unsupported models are excluded from `would_purge` and do not count as
+`cascade_failures`; supported models continue to use their existing policies.
+An empty `unsupported_models` map means no registered models were skipped for
+this reason. This reporting does not add purge support for host-defined models.
+
The window resolves in this order:
1. A host policy, when one is installed as `SOFT_DELETE_RETENTION_DAYS_FUNC`.
diff --git a/superset/tasks/deletion_retention.py
b/superset/tasks/deletion_retention.py
index c4d95a9f632..07a8cabec66 100644
--- a/superset/tasks/deletion_retention.py
+++ b/superset/tasks/deletion_retention.py
@@ -19,11 +19,11 @@
The deletion-domain analog of ``version_history.prune_old_versions``: where
that ages out version rows while keeping the live entity, this removes
entities that are already soft-deleted. For each
-``SoftDeleteMixin`` model it selects rows whose ``deleted_at`` is older than
-the per-workspace window and runs the shared cascade per entity, in bounded
-id-ordered batches. Convergent, not strictly idempotent: a re-run with the
-same clock and data removes nothing, but rows that have since crossed the
-cutoff are purged on a later run.
+``SoftDeleteMixin`` model with a purge policy it selects rows whose
+``deleted_at`` is older than the per-workspace window and runs the shared
+cascade per entity, in bounded id-ordered batches. Convergent, not strictly
+idempotent: a re-run with the same clock and data removes nothing, but rows
+that have since crossed the cutoff are purged on a later run.
"""
from __future__ import annotations
@@ -46,7 +46,10 @@ from superset.commands.deletion_retention.purge_cascade
import (
entity_uuid,
suppress_purge_association_versions,
)
-from superset.commands.deletion_retention.purge_policy import BlockerReason
+from superset.commands.deletion_retention.purge_policy import (
+ BlockerReason,
+ purge_policy_registry,
+)
from superset.commands.deletion_retention.window import
resolve_retention_window
from superset.extensions import celery_app, feature_flag_manager,
stats_logger_manager
from superset.models.helpers import (
@@ -63,8 +66,7 @@ _BATCH: int = 500
def _soft_delete_models() -> list[type[SoftDeleteMixin]]:
- """The registered ``SoftDeleteMixin`` subclasses (dashboards, charts,
- datasets), in a stable order."""
+ """Return all registered soft-delete models in a stable order."""
return list(SoftDeleteMixin._registered_subclasses) # noqa: SLF001
@@ -120,6 +122,16 @@ def _reconcile_unless_dry_run(dry_run: bool) -> None:
audit.reconcile_pending()
+def _report_model_counts(outcome: str, counts: dict[str, int]) -> None:
+ """Emit the per-model row counts for a purge outcome."""
+ entity_type: str
+ count: int
+ for entity_type, count in counts.items():
+ stats_logger_manager.instance.gauge(
+ f"{_METRIC_PREFIX}.{outcome}.{entity_type}", count
+ )
+
+
def _purge_impl(window_days: int, dry_run: bool) -> dict[str, Any]:
"""Run one purge pass across all soft-delete models."""
if window_days == 0 or window_days < -1:
@@ -141,11 +153,21 @@ def _purge_impl(window_days: int, dry_run: bool) ->
dict[str, Any]:
_reconcile_unless_dry_run(dry_run)
purged: dict[str, int] = {}
would_purge: dict[str, int] = {}
+ unsupported_models: dict[str, int] = {}
failures = 0
blocked = 0
for model in _soft_delete_models():
entity_type = _model_table_name(model)
+ if model not in purge_policy_registry():
+ unsupported_models[entity_type] = 1
+ logger.warning(
+ "deletion_retention: skipping %s: no purge policy", entity_type
+ )
+ stats_logger_manager.instance.incr(
+ f"{_METRIC_PREFIX}.unsupported_models.{entity_type}"
+ )
+ continue
purged_n, would_n, failed_n, blocked_n = _purge_model(model, cutoff,
dry_run)
if would_n:
would_purge[entity_type] = would_n
@@ -155,17 +177,15 @@ def _purge_impl(window_days: int, dry_run: bool) ->
dict[str, Any]:
blocked += blocked_n
if dry_run:
- for entity_type, count in would_purge.items():
- stats_logger_manager.instance.gauge(
- f"{_METRIC_PREFIX}.would_purge.{entity_type}", count
- )
+ _report_model_counts("would_purge", would_purge)
logger.info("deletion_retention: DRY RUN would_purge=%s", would_purge)
- return {"dry_run": 1, "would_purge": would_purge}
+ return {
+ "dry_run": 1,
+ "would_purge": would_purge,
+ "unsupported_models": unsupported_models,
+ }
- for entity_type, count in purged.items():
- stats_logger_manager.instance.gauge(
- f"{_METRIC_PREFIX}.purged.{entity_type}", count
- )
+ _report_model_counts("purged", purged)
if failures:
stats_logger_manager.instance.incr(f"{_METRIC_PREFIX}.cascade_failures")
if blocked:
@@ -176,6 +196,7 @@ def _purge_impl(window_days: int, dry_run: bool) ->
dict[str, Any]:
"purged": purged,
"cascade_failures": failures,
"blocked_by_reference": blocked,
+ "unsupported_models": unsupported_models,
}
logger.info("deletion_retention: %s", stats)
return stats
diff --git a/tests/unit_tests/tasks/test_deletion_retention.py
b/tests/unit_tests/tasks/test_deletion_retention.py
index 3608b1ca2d3..a70d4d62f3f 100644
--- a/tests/unit_tests/tasks/test_deletion_retention.py
+++ b/tests/unit_tests/tasks/test_deletion_retention.py
@@ -23,11 +23,14 @@ is preserved as the disable value, and malformed supplied
values defer purge.
import runpy
from datetime import datetime, timedelta
from pathlib import Path
-from typing import Any
-from unittest.mock import MagicMock, patch
+from typing import Any, ClassVar
+from unittest.mock import ANY, call, MagicMock, patch
+from uuid import UUID, uuid4
import pytest
+import sqlalchemy as sa
from flask.config import Config
+from sqlalchemy.orm import Mapped, mapped_column, registry, Session
@pytest.fixture
@@ -235,6 +238,170 @@ def test_clock_uses_now_not_utcnow() -> None:
purge.assert_called_once_with(Slice, now - timedelta(days=30), False)
[email protected]("dry_run", [False, True])
[email protected]("row_count", [0, 1, 3])
+def test_unsupported_model_is_reported_without_scanning(
+ app_config: Config,
+ monkeypatch: pytest.MonkeyPatch,
+ caplog: pytest.LogCaptureFixture,
+ dry_run: bool,
+ row_count: int,
+) -> None:
+ """Skip unsupported models once per run, regardless of eligible row
count."""
+ # avoid app-init regression: model helpers require the app_config fixture
first.
+ from superset.models.helpers import SoftDeleteMixin
+ from superset.tasks import deletion_retention as task
+
+ monkeypatch.setattr(SoftDeleteMixin, "_registered_subclasses", [])
+ mapper_registry: registry = registry()
+
+ @mapper_registry.mapped
+ class UnsupportedModel(SoftDeleteMixin):
+ """A host-style model with the legacy integer scan key but no purge
policy."""
+
+ __tablename__: str = "unsupported_purge_test"
+ __table__: ClassVar[sa.Table]
+ id: Mapped[int] = mapped_column(primary_key=True)
+ uuid: Mapped[UUID] = mapped_column(sa.Uuid, default=uuid4)
+
+ engine: sa.Engine = sa.create_engine("sqlite://")
+ monkeypatch.setitem(app_config, "SOFT_DELETE_PURGE_DRY_RUN", dry_run)
+ try:
+ mapper_registry.metadata.create_all(engine)
+ session: Session
+ with Session(engine) as session:
+ if row_count:
+ session.execute(
+ sa.insert(UnsupportedModel),
+ [
+ {
+ "id": index + 1,
+ "deleted_at": datetime.now() - timedelta(days=90),
+ }
+ for index in range(row_count)
+ ],
+ )
+ session.commit()
+ scan: MagicMock
+ rollback: MagicMock
+ write_ahead: MagicMock
+ counter: MagicMock
+ gauge: MagicMock
+ with (
+ patch.object(task.db, "session", session),
+ patch.object(session, "execute", wraps=session.execute) as
scan,
+ patch.object(session, "rollback", wraps=session.rollback) as
rollback,
+ patch.object(task.audit, "reconcile_pending"),
+ patch.object(task.audit, "write_ahead") as write_ahead,
+ patch.object(task, "resolve_retention_window",
return_value=30),
+ patch.object(
+ task.feature_flag_manager, "is_feature_enabled",
return_value=True
+ ),
+ patch.object(task.stats_logger_manager.instance, "incr") as
counter,
+ patch.object(task.stats_logger_manager.instance, "gauge") as
gauge,
+ ):
+ result: dict[str, Any] = task.purge_soft_deleted.run()
+ if dry_run:
+ assert result["would_purge"] == {}, result
+ else:
+ assert result["cascade_failures"] == 0, result
+ assert result["purged"] == {}, result
+ assert result["blocked_by_reference"] == 0, result
+ assert result["unsupported_models"] == {"unsupported_purge_test":
1}
+ scan.assert_not_called()
+ rollback.assert_not_called()
+ write_ahead.assert_not_called()
+ gauge.assert_not_called()
+ counter.assert_called_once_with(
+ "deletion_retention.unsupported_models.unsupported_purge_test"
+ )
+ assert (
+ caplog.text.count("skipping unsupported_purge_test: no purge
policy")
+ == 1
+ )
+ assert (
+ session.scalar(
+
sa.select(sa.func.count()).select_from(UnsupportedModel.__table__)
+ )
+ == row_count
+ )
+ finally:
+ mapper_registry.dispose()
+ engine.dispose()
+
+
[email protected]("dry_run", [False, True])
+def test_unsupported_model_does_not_prevent_supported_models_from_purging(
+ app_config: Config,
+ monkeypatch: pytest.MonkeyPatch,
+ caplog: pytest.LogCaptureFixture,
+ dry_run: bool,
+) -> None:
+ """A skipped model must not stop the remaining supported models in the
run."""
+ # avoid app-init regression: model helpers require the app_config fixture
first.
+ from superset.models.helpers import SoftDeleteMixin
+ from superset.tasks import deletion_retention as task
+
+ supported_models: list[type[SoftDeleteMixin]] =
list(task.purge_policy_registry())
+ monkeypatch.setattr(SoftDeleteMixin, "_registered_subclasses",
supported_models[:])
+
+ class UnsupportedModel(SoftDeleteMixin):
+ """An unsupported root that must never reach the row-processing
boundary."""
+
+ __tablename__: str = "unsupported_mixed_purge_test"
+
+ # Put the unsupported model first to detect an early return or break.
+ monkeypatch.setattr(
+ SoftDeleteMixin,
+ "_registered_subclasses",
+ [UnsupportedModel, *supported_models],
+ )
+ monkeypatch.setitem(app_config, "SOFT_DELETE_PURGE_DRY_RUN", dry_run)
+ purge: MagicMock
+ counter: MagicMock
+ gauge: MagicMock
+ with (
+ patch.object(
+ task, "_purge_model", return_value=(0, 1, 0, 0) if dry_run else
(1, 0, 0, 0)
+ ) as purge,
+ patch.object(task.audit, "reconcile_pending"),
+ patch.object(task, "resolve_retention_window", return_value=30),
+ patch.object(
+ task.feature_flag_manager, "is_feature_enabled", return_value=True
+ ),
+ patch.object(task.stats_logger_manager.instance, "incr") as counter,
+ patch.object(task.stats_logger_manager.instance, "gauge") as gauge,
+ ):
+ result: dict[str, Any] = task.purge_soft_deleted.run()
+
+ assert purge.call_count == len(supported_models)
+ model: type[SoftDeleteMixin]
+ for model in supported_models:
+ assert call(model, ANY, dry_run) in purge.call_args_list
+ assert result["would_purge" if dry_run else "purged"] == {
+ "dashboards": 1,
+ "slices": 1,
+ "tables": 1,
+ }
+ outcome: str = "would_purge" if dry_run else "purged"
+ assert gauge.call_count == 3
+ gauge.assert_has_calls(
+ [
+ call(f"deletion_retention.{outcome}.dashboards", 1),
+ call(f"deletion_retention.{outcome}.slices", 1),
+ call(f"deletion_retention.{outcome}.tables", 1),
+ ],
+ any_order=True,
+ )
+ assert result["unsupported_models"] == {"unsupported_mixed_purge_test": 1}
+ counter.assert_called_once_with(
+ "deletion_retention.unsupported_models.unsupported_mixed_purge_test"
+ )
+ assert (
+ caplog.text.count("skipping unsupported_mixed_purge_test: no purge
policy") == 1
+ )
+
+
def test_default_config_purges_for_real_after_the_retention_window() -> None:
"""The shipped defaults make the docs' retention promise true.
