feat(storage): add retained artifact authorities - #610
Conversation
d6fcc2a to
c07df9a
Compare
Bind subtree facades and escaped authenticated streams to a PID-scoped reader lifetime. Drain successful callback resources, abort failed ones, and retain failed descriptor or handle cleanup for explicit retry without losing the first exception. Verified with 207 passed and 10 skipped in test/test_atomic_directory.py plus Black, isort, flake8, and diff checks.
Canonicalize schema-v4 member-object metadata in the public model layer and make SQLite validate exact persisted member digests and locator fields before publication. This gives future import planners one deterministic generation identity contract. Verified in the 333-test storage suite with adversarial raw-SQL corruption coverage.
Add an optional StreamingObjectStore capability and a LocalCAS implementation that validates expected size and digest, handles durable deduplication, and preserves producer and cleanup failure ordering before publication. Verified in the 333-test storage suite, including iterator, cancellation, reuse-race, durability, and FIFO substitution regressions.
Document the authenticated subtree, compound generation identity, and streaming CAS foundations while keeping manifest import and export milestones explicitly open.
Format the SQLite migration and direct-SQL regression calls with the repository-pinned Black 24.8.0 release. Newer Black releases leave the prior spelling unchanged, but pre-commit requires the pinned output. Verified with Black 24.8.0 and test/storage/test_sqlite_catalog.py (63 passed).
Make publication cleanup completion-aware across action-call, transition, descriptor/HANDLE reuse, retained-owner, and authenticated-file handoff boundaries. Keep callback and context-body failures primary while deferring physical cleanup to retained authority owners when a nested Python call is interrupted. Also retain CAS producer close authority until the terminal state transition is complete, then hand it off at most once. Verified on Python 3.10 and 3.12 with 225 atomic tests (10 skipped), 342 storage tests, 366 direct reader-consumer tests, 256 planner tests, and the repository-pinned Black/isort/flake8 checks.
634b94b to
a27a220
Compare
Replace CPython opcode and line tracing with a narrow transition seam so the cancellation regression is deterministic under Python 3.12 and xdist. Verified with 20 xdist repetitions, the full atomic-directory test file, Python 3.12 compilation, and the changed-file pre-commit hooks.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a1da2a152
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| digest = _validate_digest(expected_digest) | ||
| byte_size = _validate_expected_object_size(expected_size) | ||
| _require_local_cas_support() | ||
| reused = self._reuse_expected_object(digest, byte_size) |
There was a problem hiding this comment.
When a LocalCAS.provision(...) store is closed concurrently with a streaming upload, this path never enters _run_strict_operation, unlike put_bytes and put_file. If the upload has reached its producer, close() can release the strict authority and return while put_chunks continues publishing and subsequently returns a valid receipt from an already-closed store; it can also race retained-descriptor checks. Run the reuse and publication sequence under the lifecycle lock so close() cannot complete until the operation finishes.
Useful? React with 👍 / 👎.
| # install succeeds, an interrupted or persistently failing close stays | ||
| # reachable for an explicit retry instead of becoming an fd/HANDLE | ||
| # leak hidden in a dead stack frame. | ||
| _register_publication_authority_owner(self) |
There was a problem hiding this comment.
Registering every empty owner here creates a permanent strong reference when construction fails before authority installation. For example, OwnedFilePublicationAuthority.__init__ creates this owner and then raises directly if require_owned_file_publication_support() fails, without calling close; repeated publication attempts on an unsupported host therefore accumulate empty owners in _RETAINED_PUBLICATION_AUTHORITY_OWNERS indefinitely unless an unrelated caller explicitly invokes the retry API. Ensure pre-install failure paths unregister the empty owner.
Useful? React with 👍 / 👎.
1 participant
Summary
Add the retained authority and streaming prerequisites needed to execute the manifest import plans introduced by #609. This is a stacked Draft PR and does not publish snapshots or refs.
Stacked directly on #609. Keep this PR in Draft until #609 and its transitive dependency chain are merged, or the stack is explicitly restacked and reverified.
Changes
StreamingObjectStorecapability and boundedLocalCAS.put_chunkspublication with exact receipt verification and zero-consumption reuse/conflict gates.Type of Change
Testing
Validated with:
225 passed, 10 skippedintest/test_atomic_directory.py342 passed366 passed256 passedgit diff --checkChecklist