Skip to content

NAS: offline-on-tape download returns actionable 409 (not misleading 404) - #40

Merged
github-actions[bot] merged 1 commit into
masterfrom
feat/nas-offline-download
Jul 20, 2026
Merged

github-actions[bot] merged 1 commit into
masterfrom
feat/nas-offline-download

Conversation

@Amantux

@Amantux Amantux commented Jul 20, 2026

Copy link
Copy Markdown
Owner

Making the NAS experience coherent over the supported features. The download endpoint returned a bare 404 for an offline-on-tape file (record present, nothing staged/cached) — which reads as 'file is gone' when it's really 'file is offline, hydrate it'.

  • download_file: 404 only when the record is truly absent; 409 with hydrate it first (POST /virtual/hydrate) guidance when the file exists but isn't staged. Matches the browse → offline → hydrate → download journey.
  • Test: upload, drop all local copies (keep the catalog record), assert 409 + hydrate guidance; existing delete/nonexistent-404 tests unaffected.

12 upload/download integration tests green.

https://claude.ai/code/session_012K5Zjc2bggcyJtFbz2D4Pw

For a coherent NAS experience: a file that exists in the catalog but is not
staged/cached is offline on tape, not missing. download_file returned a bare 404
(reads as "gone"); it now returns 409 with guidance to hydrate first (POST
/virtual/hydrate) — while a truly-absent record still 404s. This matches the
browse -> offline -> hydrate -> download journey a NAS client expects.

Test: upload a file, drop all local copies (keep the record), assert download
returns 409 with hydrate guidance; existing 404 (deleted/nonexistent) tests
unaffected. 12 upload/download integration tests green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012K5Zjc2bggcyJtFbz2D4Pw
Copilot AI review requested due to automatic review settings July 20, 2026 19:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves the NAS download experience by distinguishing “missing record” from “record exists but content is offline,” returning an actionable HTTP 409 (with hydration guidance) instead of a misleading 404 when the file is offline-on-tape.

Changes:

  • Update download_file to return 409 Conflict with hydration guidance when a catalog record exists but no local staged/cached copy is present.
  • Add an integration test that uploads a file, removes local copies, and asserts 409 with “hydrate” guidance on download.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
openblade/api/routes_upload.py Adjusts download error handling to differentiate “missing record” (404) vs “offline/unavailable locally” (409 with guidance).
tests/integration/test_upload_download_integration.py Adds an integration regression test asserting offline downloads return 409 and include hydration guidance.

Comment on lines +392 to +403
if record is not None:
# The file exists in the catalog but is not staged/cached: it is offline
# on tape. Return an actionable 409 rather than a misleading 404 so a
# NAS client knows to hydrate it first, not that the file is gone.
state = str(record.get("status") or record.get("file_state") or "offline_on_tape")
raise HTTPException(
status_code=409,
detail=(
f"File {file_id} is {state} (offline on tape). "
"Hydrate it first (POST /virtual/hydrate), then download."
),
)
Comment on lines +246 to +268
from pathlib import Path

from openblade.api import routes_upload as ru
from openblade.bootstrap import get_context

upload = client.post(
"/api/pools/integration-pool/upload",
files={"file": ("offline.txt", b"tape-resident bytes", "text/plain")},
headers=admin_auth_headers,
)
assert upload.status_code == 200
file_id = upload.json()["file_id"]
assert client.get(f"/api/files/{file_id}/download", headers=admin_auth_headers).status_code == 200

# Simulate the file going offline: drop every local copy, keep the catalog record.
record = get_context().catalog.get_nas_file_record(file_id)
candidates = [
ru._safe_resolve(ru._staging_dir(), file_id),
ru._safe_resolve(ru._restore_dir(), file_id),
]
candidates += [
Path(str(record[key])) for key in ("cache_path", "source_path") if record and record.get(key)
]
@github-actions
github-actions Bot merged commit f13e0e9 into master Jul 20, 2026
10 of 12 checks passed
Amantux added a commit that referenced this pull request Jul 20, 2026
…#41)

Walks the real NAS user journey via the API (create pool -> upload -> download ->
offline -> download-409 -> request+run restore -> download). Building it PROVED:

- The wired part works end-to-end (test_nas_journey_upload_download_and_offline_
  detection passes): pool creation, upload, online download, offline detection
  returning the actionable 409 hydrate hint (#40).
- A real gap (strict xfail): the NAS restore executor (HydrationExecutor) is a
  record-state SIMULATOR — it marks the file online_cached but does NOT materialize
  a downloadable copy (_simulate_content returns placeholder bytes and writes no
  file), so download stays 409 after a "successful" restore. The full journey
  (restore -> downloadable original bytes) does not yet complete.

The xfail is strict, so when restore is wired to materialize real bytes from tape
(read via ltfs + write to a download-resolvable cache path, exercised through the
archive-plan/ingest path), the test flips to xpass and must be updated to assert
200 + matching checksum. Documents exactly what "fully working NAS experience"
still needs.


Claude-Session: https://claude.ai/code/session_012K5Zjc2bggcyJtFbz2D4Pw

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants