refactor: make delete_metadata a proper backend contract (spec 551)#685
Merged
Conversation
- delete_metadata: no-op returning False → raises NotImplementedError - delete_metadata_prefix: added to base class, raises NotImplementedError - clear_batch_state: implemented on base class using delete_metadata + delete_metadata_prefix, removed redundant SQLite override - FakeBackend in test_circuit_breaker: implements both methods Future backends must implement metadata deletion. The silent no-op that left stale recovery state on non-SQLite backends is eliminated.
Add tests for delete_metadata, delete_metadata_prefix, and clear_batch_state on the real SQLiteBackend implementation: - delete_metadata removes key and returns True; False when missing - delete_metadata_prefix matches prefix via LIKE, returns count; 0 when no match - clear_batch_state wipes batch_registry, recovery_state, and batch_context keys for an action while leaving unrelated keys untouched - clear_batch_state is idempotent (no-op when action has no state) These replace the prior synthetic NotImplementedError tautology tests with actual behavioral coverage of the SQL layer. Refs: spec 551
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
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.
Summary
delete_metadata: changed from silent no-op (returnsFalse) toraise NotImplementedError— future backends must implement metadata deletiondelete_metadata_prefix: added toStorageBackendbase class withraise NotImplementedErrorclear_batch_state: implemented on the base class usingdelete_metadata+delete_metadata_prefix, removing the redundantSQLiteBackendoverride (identical logic)FakeBackendintest_circuit_breaker.py: implements both new required methodsWhy
The no-op default meant non-SQLite backends silently failed to delete metadata.
RecoveryStateManager.delete()andBatchContextManager.delete_batch_context_map()calldelete_metadatadirectly — stale recovery state would persist and the next run would incorrectly resume recovery.Verification
pytest→ 7488 passed, 2 skippedruff check→ all checks passedruff format --check→ 954 files already formattedFiles changed
storage/backend.pydelete_metadataraises,delete_metadata_prefixadded,clear_batch_stateimplementedstorage/backends/sqlite_backend.pyclear_batch_stateoverridetests/unit/workflow/test_circuit_breaker.pyFakeBackendimplements both methods