This is an automated email from the ASF dual-hosted git repository. alexandrusoare pushed a commit to branch alexandrusoare/feat/host-purge-policies in repository https://gitbox.apache.org/repos/asf/superset.git
commit ee7adb9414d9cbf9cde8e208b01d95f654b48175 Author: alexandrusoare <[email protected]> AuthorDate: Fri Oct 2 14:12:05 2026 +0300 adding host purge policies --- docs/docs/using-superset/recently-archived.mdx | 13 ++- .../commands/deletion_retention/purge_policy.py | 80 ++++++++++++++- superset/config.py | 7 ++ .../deletion_retention/test_purge_policy.py | 113 +++++++++++++++++++++ 4 files changed, 209 insertions(+), 4 deletions(-) diff --git a/docs/docs/using-superset/recently-archived.mdx b/docs/docs/using-superset/recently-archived.mdx index e2347ac40d7..631cab22a7f 100644 --- a/docs/docs/using-superset/recently-archived.mdx +++ b/docs/docs/using-superset/recently-archived.mdx @@ -123,7 +123,18 @@ 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. +this reason. This reporting does not itself add purge support for host-defined +models. + +A host distribution that carries its own soft-deletable entities can supply +their purge behavior by installing `PURGE_POLICIES_FUNC`: a callable returning +one `PurgeEntityPolicy` per root it owns. Installed policies are validated +against the discovered dependency graph exactly as the built-in ones are, and a +model that gains an installed policy stops being reported as unsupported. A +provider that fails, returns something other than a sequence of policies, or +redeclares a built-in chart, dashboard or dataset root is logged and ignored, +leaving the built-in roots unchanged. Hosts can require +`superset.versioning.utils.HOST_POLICY_API_VERSION` before installing. The window resolves in this order: diff --git a/superset/commands/deletion_retention/purge_policy.py b/superset/commands/deletion_retention/purge_policy.py index 67b26646257..b36b7b11eb5 100644 --- a/superset/commands/deletion_retention/purge_policy.py +++ b/superset/commands/deletion_retention/purge_policy.py @@ -27,6 +27,7 @@ from types import MappingProxyType from typing import Any, cast, NamedTuple import sqlalchemy as sa +from flask import current_app, has_app_context from sqlalchemy.orm import Mapper, Session from superset.utils.sqlalchemy_events import ( @@ -54,6 +55,12 @@ ALL_REASON_CODES: frozenset[str] = frozenset( } ) +#: Config key holding the host's purge-policy provider: a zero-argument +#: callable returning ``PurgeEntityPolicy`` objects for soft-delete roots this +#: package does not own. Downstream startup may require +#: ``superset.versioning.utils.HOST_POLICY_API_VERSION`` before installing one. +HOST_POLICIES_CONFIG_KEY: str = "PURGE_POLICIES_FUNC" + class BlockerReason(NamedTuple): """One blocker's persisted audit code paired with its operator phrase.""" @@ -410,8 +417,8 @@ def compare_policy( @lru_cache(maxsize=1) -def purge_policy_registry() -> Mapping[type[Any], PurgeEntityPolicy]: - """Build the supported purge registry after model initialization.""" +def _builtin_purge_policies() -> tuple[PurgeEntityPolicy, ...]: + """Declare the purge roots this package owns, after model initialization.""" # avoid circular import: model listener registration imports neutral event helpers from superset.connectors.sqla.models import SqlaTable from superset.models.dashboard import Dashboard @@ -860,7 +867,74 @@ def purge_policy_registry() -> Mapping[type[Any], PurgeEntityPolicy]: cleanup_permission=cleanup_dataset_permission, ), } - return validate_unique_root_policies(registry.values()) + return tuple(registry.values()) + + +def _host_purge_policies() -> tuple[PurgeEntityPolicy, ...]: + """Return the purge policies a host installed for its own roots. + + A host distribution can carry ``SoftDeleteMixin`` entities this package + cannot import. The retention task discovers those entities through the + mixin registry, so without a policy they reach the cascade as an + unsupported model. The host therefore declares their purge behavior and + installs it under ``PURGE_POLICIES_FUNC``. + + Host boundary: an unavailable provider, a malformed payload, or a policy + that collides with a root declared here is logged and dropped. A broken + host declaration must not take the scheduled purge down with it, and must + never redefine how a chart, dashboard or dataset is purged. + """ + if not has_app_context(): + return () + provider: Callable[[], Any] | None = current_app.config.get( + HOST_POLICIES_CONFIG_KEY + ) + if provider is None: + return () + try: + provided: Any = provider() + except Exception: # pylint: disable=broad-except + logger.exception( + "purge_policy: %s is unavailable; keeping built-in roots only", + HOST_POLICIES_CONFIG_KEY, + ) + return () + if not isinstance(provided, (list, tuple)) or not all( + isinstance(policy, PurgeEntityPolicy) for policy in provided + ): + logger.error( + "purge_policy: %s returned %s; expected a sequence of PurgeEntityPolicy", + HOST_POLICIES_CONFIG_KEY, + type(provided).__name__, + ) + return () + builtin_roots: frozenset[type[Any]] = frozenset( + policy.model for policy in _builtin_purge_policies() + ) + accepted: list[PurgeEntityPolicy] = [] + for policy in provided: + if policy.model in builtin_roots: + logger.error( + "purge_policy: host policy for built-in root %s ignored", + policy.model.__name__, + ) + continue + accepted.append(policy) + return tuple(accepted) + + +def purge_policy_registry() -> Mapping[type[Any], PurgeEntityPolicy]: + """Index the built-in purge roots plus any the host installed. + + Uncached on purpose, unlike its two inputs: the built-in declarations are + built once per process, while a host policy is resolved per call so a + provider installed after the first purge is still honored. The hot path is + ``get_purge_policy``, which caches per model, so rebuilding this small + index is not on it. + """ + return validate_unique_root_policies( + (*_builtin_purge_policies(), *_host_purge_policies()) + ) @lru_cache(maxsize=None) diff --git a/superset/config.py b/superset/config.py index 1693c4b09fe..0dd97727a6b 100644 --- a/superset/config.py +++ b/superset/config.py @@ -1061,6 +1061,13 @@ SOFT_DELETE_RETENTION_DAYS: int = _parse_soft_delete_retention_days() # Optional authoritative host policy, consulted before the shared CLI override. # Invalid/unavailable results skip purge rather than fall back to a stored value. SOFT_DELETE_RETENTION_DAYS_FUNC: Callable[[], int] | None = None +# Optional host-installed purge policies, for soft-delete roots this package +# does not own. A zero-argument callable returning a sequence of +# ``superset.commands.deletion_retention.purge_policy.PurgeEntityPolicy``. +# Annotated loosely to keep config import-light. A failing or malformed +# provider, or one redeclaring a built-in root, is logged and dropped: the +# built-in chart, dashboard and dataset roots are never affected. +PURGE_POLICIES_FUNC: Callable[[], Any] | None = None SOFT_DELETE_PURGE_DRY_RUN: bool = False # Retention policy for the purge audit log itself (the durable evidence the diff --git a/tests/unit_tests/commands/deletion_retention/test_purge_policy.py b/tests/unit_tests/commands/deletion_retention/test_purge_policy.py index 2b9582fcd7c..35f74832a28 100644 --- a/tests/unit_tests/commands/deletion_retention/test_purge_policy.py +++ b/tests/unit_tests/commands/deletion_retention/test_purge_policy.py @@ -18,12 +18,15 @@ from __future__ import annotations +from collections.abc import Callable, Iterator +from contextlib import contextmanager from dataclasses import replace from typing import Any from unittest.mock import MagicMock import pytest import sqlalchemy as sa +from flask import current_app from sqlalchemy.engine import Dialect from sqlalchemy.orm import configure_mappers, registry from sqlalchemy.sql import Select @@ -40,6 +43,7 @@ from superset.commands.deletion_retention.purge_policy import ( DependencyPolicy, discover_dependencies, get_purge_policy, + HOST_POLICIES_CONFIG_KEY, listener_responsibilities, PolicyCoverage, purge_policy_registry, @@ -568,3 +572,112 @@ def test_core_delete_actions_compile_for_supported_dialects(dialect: str) -> Non assert compiled assert all(statement.startswith(("SELECT", "DELETE")) for statement in compiled) + + +def _host_root(prefix: str) -> type[Any]: + """Map a throwaway root in its own ``MetaData``. + + The hook is indifferent to a host root's shape, so this is the smallest + graph with anything to declare: one outbound foreign key, the same edge + every built-in policy preserves for its ``ab_user`` columns. + """ + metadata: sa.MetaData = sa.MetaData() + sa.Table( + f"{prefix}_owner", + metadata, + sa.Column("id", sa.Integer, primary_key=True), + ) + root_table: sa.Table = sa.Table( + f"{prefix}_entity", + metadata, + sa.Column("id", sa.Integer, primary_key=True), + sa.Column("owner_id", sa.Integer, sa.ForeignKey(f"{prefix}_owner.id")), + sa.Column("deleted_at", sa.DateTime, nullable=True), + ) + + class HostRoot: + """Temporary mapped root standing in for an entity core does not own.""" + + registry().map_imperatively(HostRoot, root_table) + return HostRoot + + +def _host_edge(prefix: str) -> DependencyPolicy: + """The one dependency ``discover_dependencies`` finds for that root.""" + return DependencyPolicy( + DependencyKey( + "foreign_key", + f"{prefix}_entity", + f"{prefix}_owner", + ("owner_id",), + ("id",), + "outbound", + ), + DependencyClassification.PRESERVE, + ) + + +def _host_policy( + model: type[Any], dependencies: tuple[DependencyPolicy, ...] +) -> PurgeEntityPolicy: + """Borrow the chart policy's executable actions for a host root.""" + return replace( + get_purge_policy(Slice), + model=model, + entity_type="host_root", + dependencies=dependencies, + ) + + +@contextmanager +def _installed(provider: Callable[[], Any]) -> Iterator[None]: + """Install a host policy provider for the duration of one test.""" + previous: Any = current_app.config.get(HOST_POLICIES_CONFIG_KEY) + current_app.config[HOST_POLICIES_CONFIG_KEY] = provider + try: + yield + finally: + current_app.config[HOST_POLICIES_CONFIG_KEY] = previous + + +def test_host_policy_extends_the_registry() -> None: + """A complete host declaration resolves like a built-in root.""" + model: type[Any] = _host_root("extend") + policy: PurgeEntityPolicy = _host_policy(model, (_host_edge("extend"),)) + + with _installed(lambda: [policy]): + assert get_purge_policy(model) is policy + assert {Slice, Dashboard, SqlaTable} <= set(purge_policy_registry()) + + +def test_host_policy_is_held_to_the_discovered_graph() -> None: + """An incomplete host declaration is rejected, not silently honored.""" + model: type[Any] = _host_root("incomplete") + + with _installed(lambda: [_host_policy(model, ())]): + with pytest.raises(RuntimeError, match="Incomplete purge policy"): + get_purge_policy(model) + + [email protected]( + "provider", + [ + pytest.param(lambda: 1 // 0, id="raises"), + pytest.param(lambda: "not-a-sequence", id="wrong_type"), + pytest.param(lambda: [object()], id="wrong_member"), + ], +) +def test_malformed_host_provider_leaves_builtin_roots_intact( + provider: Callable[[], Any], +) -> None: + """A broken host boundary costs the host its roots, not the purge.""" + with _installed(provider): + assert set(purge_policy_registry()) == {Slice, Dashboard, SqlaTable} + + +def test_host_policy_cannot_redeclare_a_builtin_root() -> None: + """A host cannot redefine how a chart, dashboard or dataset is purged.""" + builtin: PurgeEntityPolicy = get_purge_policy(Slice) + + with _installed(lambda: [replace(builtin, dependencies=())]): + assert purge_policy_registry()[Slice] is builtin
