fix(deletion-retention): dedupe repeated blocked audits - #42863
fix(deletion-retention): dedupe repeated blocked audits#42863mikebridge wants to merge 2 commits into
Conversation
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #8e2f54
Actionable Suggestions - 2
-
superset/tasks/deletion_retention.py - 1
- Missing retained disposition metric · Line 283-293
-
superset/migrations/versions/2026-08-06_18-00_b8d2f4a6c901_index_purge_audit_predecessor.py - 1
- Migration lineage break · Line 29-29
Filtered by Review Rules
Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
-
tests/integration_tests/deletion_retention/audit_tests.py - 1
- Deprecated datetime.utcnow() in tests · Line 213-213
Review Details
-
Files reviewed - 7 · Commit Range:
9ef7504..9ef7504- superset/commands/deletion_retention/audit.py
- superset/migrations/versions/2026-08-06_18-00_b8d2f4a6c901_index_purge_audit_predecessor.py
- superset/models/purge_audit_log.py
- superset/tasks/deletion_retention.py
- tests/integration_tests/deletion_retention/audit_tests.py
- tests/integration_tests/deletion_retention/force_purge_tests.py
- tests/integration_tests/deletion_retention/purge_tests.py
-
Files skipped - 1
- UPDATING.md - Reason: Filter setting
-
Tools
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
richardfogaca
left a comment
richardfogaca
left a comment
There was a problem hiding this comment.
Richard's agent here:
Blocking: _retention_predecessor() selects one row ordered only by created_on, which is not a unique sequence. Multiple prior scheduled outcomes can share the newest timestamp; if a tied blocked row is selected while another tied row is failed or confirmed, the current audit is suppressed even though the preceding outcome is ambiguous.
Could we retain the current row whenever the maximum predecessor timestamp is tied—or introduce a durable ordering key—and cover tied blocked plus non-blocked predecessors with a regression test?
Reviewed at 6f6c61e. The current provider status exposes only documentation-preview checks, not the advertised backend test and migration validation.
Code Review Agent Run #b3e811Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
3 participants
SUMMARY
Deduplicate consecutive scheduled retention audits that remain blocked for the same entity while preserving the write-ahead audit invariant. The scheduled purge still writes a provisional
pendingrecord before evaluating the cascade, but removes only that current provisional record when its unambiguous, strictly older scheduled predecessor is alreadyblocked.The change preserves completed audit history, retains evidence on ambiguous ordering or persistence uncertainty, and leaves force-purge auditing independent. It also adds suppression/fallback metrics and a composite predecessor-lookup index.
Shortcut: SC-115343.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable; backend audit behavior only.
TESTING INSTRUCTIONS
Expected: 61 tests pass.
The migration was verified on an isolated SQLite metadata database with an upgrade, downgrade to
c4a1b8e2d739, and re-upgrade.EXPLAIN QUERY PLANselectedix_purge_audit_log_retention_predecessorfor the predecessor lookup. Changed-file pre-commit passes, including MyPy, Ruff, and Pylint.ADDITIONAL INFORMATION
The migration adds one non-unique index to
purge_audit_log. The table and scheduled purge were introduced recently, and each normal run processes at most 500 eligible entities per batch, so existing deployments are expected to have audit histories on the order of thousands—not millions—of rows unless the task has been invoked unusually often. At that expected size, a normal index build should complete quickly and the portability of Superset’s transactional migration helper is preferable to a PostgreSQL-only concurrent-index path. Runtime remains proportional to the actual audit-table row count. PostgreSQL may block writes and MySQL may hold a metadata lock during creation; operators with unexpectedly large audit tables should measure the table and schedule the migration in a maintenance window. No application downtime is otherwise expected. The downgrade removes only this index.