Repository navigation
uploadDataset silently skips a series manifest's bytes (uploaded doc references a nonexistent tempfile after DID ingests) #306
Description
Surfaced by the round-trip test PR #305 added for #304, in scheduled cloud run 34924762494 against TEST_USER_1. Both test_members_survive_a_download_from_the_cloud and ..._with_sync_files_false fail on the download side with:
Refusing to add document "...": series "chunkdata.bin" declares 4 members but records no ingest_locations.
Downstream of a real upload-side bug the round-trip test caught for the first time.
What is going wrong
The captured cloud-side signal in the failed run:
CloudAPIError: HTTP 409 Conflict - Manifest for file series "chunkdata.bin" (uid <manifest_uid>) is not a file of this dataset
No file details for <manifest_uid> in dataset <cloud_id>: Not found (HTTP 404): {"message":"File not found"}
Cannot fetch manifest for series "chunkdata.bin" (uid <manifest_uid>): Not found (HTTP 404)
Local file does not exist for uid <manifest_uid> at .../download/files/<manifest_uid>
The server has no record of the manifest uid on the freshly uploaded dataset. Neither reconstruction path can succeed:
- SyncFiles=true:
downloadDatasetFiles404s on the manifest uid, sostaging/<manifest_uid>never gets written, andupdateFileInfoForLocalFiles->reconstructSeriesIngestLocationssilently skips (not os.path.isfile(manifest_path)), leavingingest_locationsempty. - SyncFiles=false: my Rebuild series ingest_locations on SyncFiles=false download (companion to VH-Lab/NDI-matlab#988) #302 rebuild path tries to fetch the manifest through the customFileHandler; the batch signed-URL scope returns a 409 that names the exact server-side complaint. Silently skips,
ingest_locationsstays empty.
DID's MembersNotLocatable guard then refuses the doc on add_docs. The bytes were never on the cloud; they never had a chance.
Root cause
ndi.cloud.upload.file_uploads_for_document(doc) walks files.file_info[].locations[] and yields (uid, local_path) pairs to upload, requiring os.path.exists(candidate) on the recorded location.
did.document.Document.add_file_series(name, member_paths) writes the manifest to tempfile.NamedTemporaryFile(prefix="did_manifest_", suffix=".manifest", delete=False), and calls add_file(name, manifest_path) — which stores manifest_path as the location and, for a file-type location, defaults delete_original=True. database_add runs the ingest, DID copies the manifest into <FileDir>/<uid> and deletes the source. The file_info.locations[0].location still holds the (now-gone) tempfile path.
By the time uploadFilesForDatasetDocuments runs, that tempfile is gone. file_uploads_for_document returns no pair for the manifest slot, the upload silently skips it, and only the metadata reaches the cloud. The next download 404s.
Reproduced locally:
=== BEFORE database_add ===
name: chunkdata.bin
uid: 41269720e0a8f771_...
location: /tmp/did_manifest_kgwlwr83.manifest
exists: True
=== AFTER database_add ===
location: /tmp/did_manifest_kgwlwr83.manifest
exists: False <-- gone
=== check DID FileDir ===
cached file: 41269720e0a8f771_... 189 bytes <-- bytes still on disk under the uid
This is also a latent bug for any ordinary file where the user-provided source path is later moved or deleted before the upload: the ingest already copied the bytes into DID's file store, but the upload can't find them again.
What to fix
Teach the upload path to fall back to DID's file cache when the recorded location no longer resolves. The bytes are keyed by uid inside <FileDir>/<uid> (and DID's global filecachepath), and did.file.cached_path_for_uid(uid, additional_roots=[...]) already does exactly this lookup for the read path.
- Extend
file_uploads_for_document(doc, additional_roots=None): after the recorded-location probe, if the location's file does not exist, look the uid up in the given roots viacached_path_for_uid. - Extend
uploadFilesForDatasetDocuments(..., additional_roots=None)to forward it. - Callers (
orchestration.uploadDatasetandsync.operations._upload_binaries) resolve the local dataset'sbinary_path— DID'sFileDir— and pass it in.
Not proposed: changing DID's add_file_series to use a persistent path instead of a tempfile. That would change DID semantics for a symptom this fix already covers.
Not proposed: rewriting file_info.locations[].location to the ingested path on the way through DID. That is a broader change with more ripples.
Regression tests
Offline unit tests in tests/test_cloud_upload_files_from_did_cache.py (or similar):
file_uploads_for_documentreturns a(uid, path)pair when the recorded location is gone but the file is at<additional_roots[0]>/<uid>.- Same for a MATLAB-shaped file_info (one-element
locationsas a bare dict). - If neither the recorded location nor the cached one exists, no pair is yielded (nothing to upload; the caller records the doc as
failed_document_idsas it does today). - If the recorded location DOES exist, it is preferred and the cache is not consulted (test data is separable, so preferring the recorded path avoids reading from DID's cache when the source is still there).
- Cross-checked against a real
database_addof ademoNDISeriesdoc: after add,file_uploads_for_document(doc, additional_roots=[binary_path])yields the manifest's(uid, cached_path).
The end-to-end coverage is PR #305's live-cloud test — which will pass once this fix lands.
References
- Live-cloud round-trip test: a downloaded series' members must be readable (SyncFiles=true and =false) #304 (the round-trip test)
- Live-cloud file-series round-trip test (#304) #305 (the round-trip test's PR; caught this on TEST_USER_1)
- Actions run 34924762494 — the scheduled run against TEST_USER_1 where this surfaced
- Rebuild series ingest_locations on SyncFiles=false download (companion to VH-Lab/NDI-matlab#988) #302 / PR Rebuild series ingest_locations on SyncFiles=false download (#302) #303 (the offline SyncFiles=false rebuild — works, but it needs the manifest to be reachable on the cloud, which is the bug this issue names)
- Retire the manifest cloud-reference workaround and add an offline SyncFiles=false round-trip test once DID-matlab#201 lands VH-Lab/NDI-matlab#986 (parent tracking issue on the file-series cloud work)
Activity