Repository navigation
NAS restore materializes real bytes; full NAS journey closes end-to-end - #42
Merged
Merged
Conversation
HydrationExecutor previously marked a restored file online_cached but wrote
no downloadable content (placeholder bytes only), so download stayed 409 after
a "successful" restore. Now _materialize_content reads the original archived
bytes back from tape (ltfs.read_bytes at the /<dataset>/<relpath> path ingest
used) and writes a real cache file at restore_dir/{file_id}, which the download
endpoint resolves. Falls back to the placeholder for never-archived records.
- hydration.py: add _read_archived_bytes + _materialize_content; _simulate_content
still the patchable per-file hook (now tape-backed).
- import_guard.py: allow hydration.py (restore data path uses ltfs.read_bytes).
- e2e journey test: flip the restore step from strict-xfail to a passing full
journey — upload -> archive-to-tape -> offline(409) -> restore -> download 200
with matching sha256.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012K5Zjc2bggcyJtFbz2D4Pw
Addresses reviewer blocker on the restore-materialization change: a failed or placeholder tape read was written to the cache and served by download as a checksum-clean 200 with wrong bytes (the download endpoint does not re-verify). - hydration.py: _materialize_content now verifies materialized bytes against the record's recorded sha256 before caching. Enforced only when the record carries a real 64-hex digest (production upload/ingest always do), so a corrupt/placeholder restore raises ChecksumMismatchError and the file is marked FAILED instead of served. Simulator records with sentinel checksums keep the placeholder path, so the executor stays a pure record-state simulator for them. - tape_paths.py (new): single dataset_tape_path() helper; ingest (write) and hydration (read-back) now both use it, so the on-tape key cannot drift apart — the drift class the reviewer flagged is gone by construction. The e2e journey test writes via the same helper. - test: regression test proving a real-checksum record with no matching tape bytes is FAILED, not reported restored (safety-relevant bug ships with its test). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012K5Zjc2bggcyJtFbz2D4Pw
The materialization change overwrote cache_path with the physical restore-cache
file, breaking the recall contract (clients/tests read cache_path as the logical
destination they requested, e.g. /openblade/fast-recall/...). Download resolves the
real bytes at restore_dir/{file_id} independently of cache_path, so keep cache_path
logical and let _materialize_content own the physical file. Fixes the 3 i3
end-to-end archive-recall regressions.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012K5Zjc2bggcyJtFbz2D4Pw
There was a problem hiding this comment.
Pull request overview
This PR closes the NAS “full journey” gap by making the NAS restore executor actually materialize archived bytes from tape into the restore cache so downloads work again (and are checksum-verified), with shared tape-path derivation to prevent ingest/restore drift.
Changes:
- Hydration now reads bytes back from tape, writes a real restored cache file, and verifies restored content against the recorded SHA-256 before marking a file restored.
- Introduces a shared
dataset_tape_path()helper so ingest (write) and hydration (read) use the same on-tape key layout. - Updates end-to-end and unit tests to validate the upload → offline → restore → download journey and to regression-test checksum-gated failure behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
openblade/nas/hydration.py |
Reads archived bytes from tape, materializes restore cache content, and checksum-verifies restored bytes. |
openblade/nas/ingest.py |
Uses the shared tape-path helper for simulator writes to keep tape key layout consistent. |
openblade/nas/tape_paths.py |
Adds a single canonical helper for NAS on-tape path derivation. |
openblade/safety/import_guard.py |
Allowlists hydration module for SAFETY_003 since restore reads from tape (non-destructive). |
tests/e2e/test_nas_journey.py |
Converts the prior xfail into a passing full journey test with checksum verification. |
tests/unit/test_nas_hydration.py |
Adds a safety regression ensuring checksum-mismatch restores fail rather than serving placeholder bytes. |
Comment on lines
+15
to
+17
| def dataset_tape_path(dataset_name: str, relative_path: str) -> PurePosixPath: | ||
| """Return the on-tape path for a file: ``/<dataset name>/<relative path>``.""" | ||
| return PurePosixPath("/") / dataset_name / relative_path |
Comment on lines
+325
to
+329
| cache_dir = Path(os.environ.get("OPENBLADE_RESTORE_DIR", "/tmp/openblade-restore")) | ||
| cache_dir.mkdir(parents=True, exist_ok=True) | ||
| cache_file = cache_dir / record.id | ||
| with cache_file.open("wb") as handle: # download resolves restore_dir/{file_id} | ||
| handle.write(content) |
Comment on lines
+342
to
+346
| expected = record.checksum_sha256 | ||
| if not expected or not _SHA256_RE.match(expected): | ||
| return | ||
| actual = hashlib.sha256(content).hexdigest() | ||
| if actual != expected: |
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.
What
The NAS restore executor (
HydrationExecutor) previously marked a restored fileonline_cachedbut wrote no downloadable content —_simulate_contentreturned placeholder bytes and no file, sodownloadstayed 409 after a "successful" restore. The full NAS journey (upload → offline → restore → download) could not close.This makes restore materialize the real archived bytes read back from tape, so a restored file is byte-identical to the original and downloadable again.
Changes
hydration.py—_materialize_contentreads the original bytes back from tape (ltfs.read_bytesat the/<dataset>/<relpath>path ingest wrote), writes a real cache file atrestore_dir/{file_id}that the download endpoint resolves, and verifies the bytes against the record's recorded sha256 before serving. Placeholder fallback is retained only for never-archived / simulator records (sentinel checksums).tape_paths.py(new) — onedataset_tape_path()helper; ingest (write) and hydration (read-back) both use it, so the on-tape key cannot drift apart.import_guard.py—hydration.pyadded to the SAFETY_003 allowlist (restore reads tape vialtfs.read_bytes; non-destructive).Safety (T3)
Touches the NAS restore data path and the SAFETY_003 allowlist. Reviewed by the adversarial reviewer subagent; its data-integrity blocker (a failed tape read served as a wrong-bytes 200) is fixed by the checksum-gated verification above and covered by a regression test.
Verified
make lintclean on changed files; 117 restore/archive/ingest/recall tests green (incl. the 3 i3 end-to-end hash-verified recalls); hydration + journey suites green.🤖 Generated with Claude Code
https://claude.ai/code/session_012K5Zjc2bggcyJtFbz2D4Pw