Repository navigation
feat(reporting): schedule retention in the production worker - #1476
Merged
Merged
Conversation
ReportingProductionSupport runs the retention a store opted into every maintenance_seconds (default 300, None disables), as adcontextprotocol/adcp#7996 section 4.3 asks of the production service: intent sweeps for row storage, period retirement for row storage or the change feed, and change pruning for the feed. Each operation is bounded and resumable. A failure never stops the worker; only the closed operation name is logged. Part of #1457. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean, additive PR with no blocking or medium findings.
Checked:
_maintainschedules opt-in retention (sweep/retire/prune) on the worker loop; opt-in logic (service.py:123-130), per-operation failure isolation, and result-type handling are correct.getattr(store, "_row_storage"/"_change_feed", default)safely no-ops on the in-memory store;prune_changesreturningintnever trips the.failedcheck._maintenance_failedmirrors the sanitized_worker_stoppedsignal; leak test confirms no exception/secret reaches the record.- Ops run inline without asyncio timeouts, but
lock_timeoutbounds the real blocking risk and failures retry next interval — consistent with the existing loop. - Semver
feat:is the correct signal for an additive optional param; no public-surface breaks, no generated-code hand-edits, no import-layering issues.
No Critical/High/Medium findings. Not high-risk, no gated paths, no no-auto-approve team match. Falls through to row 9.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #1457 (Stage 2 of adcontextprotocol/adcp#7996, section 4.3). The last PR of the stack; builds on the merged retention (#1469), change feed (#1472) and period retirement (#1474).
Change
ReportingProductionSupportruns the retention a store opted into everymaintenance_seconds(default 300;Nonedisables it), on the same worker loop as the producer and materializer:row_storagesweep_row_write_intentsrow_storageorchange_feedretire_expired_periodschange_feedprune_changesEach operation is bounded and resumable. A failing operation never stops the worker: the next interval retries it, and only the closed operation name (
REPORTING_MAINTENANCE_FAILEDwithintent_sweep,period_retirementorchange_pruning) is logged, never an exception, provider message or identifier. An operation that finishes but reports failed items (a binding that cannot delete, for example) is logged the same way, so a stuck binding is visible; the stable per-item codes are in the result'scauses.retire_expired_periodsalso deletes the row bytes of every period it retires, soprune_expired_row_setsis not scheduled; call it yourself only to retire row bytes earlier than whole periods.Locks
No new locks: the scheduled operations take the locks audited in #1467 (Python account lock, object lock aligned with the JS SDK).
Tests
tests/test_reporting_production_maintenance.py: only opted-in operations run, the interval and the disable switch, a raising operation never stops the others or leaks, and per-item failures in a result are logged by operation name.Merged by the repo owner's watcher without waiting for code-owner review; @rachitm022 is requested for visibility and will test the finished stack.
🤖 Generated with Claude Code