mikebridge commented on code in PR #42863:
URL: https://github.com/apache/superset/pull/42863#discussion_r3736197404
##########
superset/commands/deletion_retention/audit.py:
##########
@@ -182,6 +197,146 @@ def block(record_id: UUID | None) -> None:
finalize(record_id, STATUS_BLOCKED)
+def _capture_recovery_snapshot(record: PurgeAuditLog) ->
_AuditRecoverySnapshot:
+ """Capture the content-free fields needed for fail-safe recovery."""
+ return _AuditRecoverySnapshot(
+ id=cast(UUID, record.id),
+ actor=str(record.actor),
+ entity_type=str(record.entity_type),
+ entity_uuid=record.entity_uuid,
+ created_on=cast(datetime, record.created_on),
+ )
+
+
+def _retention_predecessor(
+ session: Session, current: PurgeAuditLog
+) -> PurgeAuditLog | None:
+ """Return the latest row that could unambiguously precede ``current``."""
+ return session.execute(
+ sa.select(PurgeAuditLog)
+ .where(PurgeAuditLog.entity_uuid == current.entity_uuid)
+ .where(PurgeAuditLog.entity_type == current.entity_type)
+ .where(PurgeAuditLog.trigger == TRIGGER_RETENTION)
+ .where(PurgeAuditLog.created_on <= current.created_on)
+ .where(PurgeAuditLog.id != current.id)
+ .order_by(PurgeAuditLog.created_on.desc())
+ .limit(1)
+ ).scalar_one_or_none()
Review Comment:
Thanks for raising the overlap case. The pending-predecessor behavior is
intentional and covered by test_pending_predecessor_retains_current_block:
while the predecessor is pending, deleting the current row could erase the only
blocked evidence if that predecessor later fails, is reconciled differently, or
its commit outcome is uncertain. The audit protocol therefore fails safe by
retaining the current row. A later sequential blocked attempt collapses the
overlap, as pinned by
test_overlap_duplicates_do_not_cause_unbounded_sequential_growth. Strict
serialization would require a portable per-entity lock or schema-level claim
and would broaden this audit-noise fix while adding deadlock/availability risk
across PostgreSQL, MySQL/MariaDB, and SQLite. I am keeping the
evidence-preserving tradeoff rather than making suppression correctness depend
on locking. Separately, the failed MySQL CI job exposed DATETIME(0) timestamp
truncation in sequential attempts; the latest commit migrates My
SQL/MariaDB created_on to DATETIME(6), and the three exact failed tests now
pass on local MySQL 8.
--
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]
