mikebridge opened a new pull request, #43485: URL: https://github.com/apache/superset/pull/43485
### SUMMARY Blocked purge audit records currently record only a status: an auditor reading `purge_audit_log` cannot distinguish "blocked because an alert/report references the entity" from "blocked because a user has the dashboard as their welcome page" from "failed on a restrictive foreign key the cascade does not handle" — outcomes with entirely different remediations. The reason is already known at decision time (logs, CLI output) but is discarded before it reaches the durable record. This PR threads a **stable machine-readable reason code** from the purge decision points into the audit record, and amends the #42863 growth-suppression predicate to key on **status and reason**: - **Vocabulary**: `REASON_REPORT_SCHEDULE` (`"report_schedule"`), `REASON_USER_ATTRIBUTE` (`"user_attribute"`), and `REASON_UNHANDLED_REFERENCE` (`"unhandled_reference"`) declared in `purge_policy.py` (the vocabulary's owner; deliberately model-free). The values are frozen identifiers pinned by a golden-set test — a future physical table rename must not re-mint persisted history. `PurgeBlockedError` carries the code alongside its human phrase; the first declared blocker that matches wins (declaration order is part of the contract, unit- and integration-tested). - **Threading**: `CascadeResult.blocked_reason_code` set by both cascade handlers (policy block from the exception; `IntegrityError` → `unhandled_reference`); `audit.block()` and `audit.finalize_retention_blocked()` take the reason as a required parameter, carried through the retain path and **both** crash-recovery branches as a parameter (never read from the reason-less pending row). - **Reading rule**: an unhandled-FK failure keeps `blocked` status (so growth suppression continues to apply) — `status = blocked AND reason = unhandled_reference` denotes a cascade-coverage defect, not an intended policy outcome. Run-level metrics conflate this class with reference blocks, making the persisted code the sole durable classifier. - **Suppression amendment**: same-status-same-reason nights stay suppressed; a reason change writes exactly one new blocked row carrying the new code. The predecessor is the **latest** same-entity retention row overall (clock skew or an overlapping run can no longer suppress against a non-latest row — the newer row fails the strictly-older check and the attempt is retained, fail-safe). NULL-safe both ways: a reason-less (pre-upgrade) predecessor never matches, and a missing current code never suppresses (WARNING, fail-open). Timestamp-tie ambiguity widens to "differs in status **or** reason" via `is_distinct_from` (NULL-safe on PostgreSQL/MySQL/SQLite). - **Strictly reason-less**: failed, confirmed, race-lost, and reconciled rows never carry a reason. The known crash window (worker dies between the blocked decision and finalization; `reconcile_pending` later finalizes failed/NULL) is documented and deliberately not "fixed" by fabricating a code — the next night re-anchors with the real one. - **Migration**: one additive nullable `String(64)` column via the shared migration utils. No backfill, no index, no reader changes; historical rows keep NULL and every reader tolerates absence. The #42863 suppression paragraph in `UPDATING.md` is amended to state the new predicate. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF Backend-only. Before/after of the audit table for the same three blocked outcomes: ``` -- BEFORE status | reason blocked | (column does not exist) blocked | blocked | -- AFTER status | reason blocked | report_schedule blocked | user_attribute blocked | unhandled_reference ``` ### TESTING INSTRUCTIONS ```bash # unit: vocabulary golden set, threading, first-match pytest tests/unit_tests/commands/deletion_retention/ tests/unit_tests/tasks/test_deletion_retention.py # integration: three-way distinction, suppression contract incl. reason-change, # NULL-safety, latest-predecessor ordering, recovery branches, immutability pytest tests/integration_tests/deletion_retention/ # migration round-trip (verified on SQLite and PostgreSQL) superset db upgrade && superset db downgrade 1072de5ed955 && superset db upgrade ``` Manual: soft-delete a chart referenced by an active report, run the retention purge (or `superset deletion-retention force-purge`), then `SELECT status, reason FROM purge_audit_log` — the blocked row carries `report_schedule`. The changed-reason suppression test was control-run: it fails against the pre-amendment status-only predicate and passes with this PR. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [x] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [x] Migration is atomic, supports rollback & is backwards-compatible - [x] Confirm DB migration upgrade and downgrade tested - [x] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API Runtime: metadata-only DDL on PostgreSQL/SQLite; instant DDL on MySQL 8.0.12+ (trailing nullable ADD COLUMN); table rebuild only on older MySQL — `purge_audit_log` is small and growth-suppressed by design. No downtime expected. Single alembic head verified (`1072de5ed955`) before authoring. Internal tracking: sc-115342. Follows #42863 (audit dedupe) and #42888 (declarative purge policies). *This PR was generated by Claude (AI) on behalf of @mikebridge.* 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
