Repository navigation
Rebuild series ingest_locations on SyncFiles=false download (companion to VH-Lab/NDI-matlab#988) #302
Description
Follow-up carved out of the closing note on #300. The workaround retirement was retracted, but this gap survived and is worth its own tighter issue.
What is missing
ndi.cloud.filehandler.updateFileInfoForRemoteFiles rewrites a downloaded document's file_info locations to ndic:// URIs on a SyncFiles=false download. It does not rebuild series_info[k].ingest_locations for any series the document carries. DID strips ingest_locations before storing a document, so a document read back from a cloud dataset returns with n_present populated and ingest_locations empty -- exactly the shape DID's MembersNotLocatable guard (DID-matlab#185 / DID-python's _reject_series_without_ingest_locations) refuses on the following add_docs. The whole SyncFiles=false path is blocked on that guard for any document carrying a populated series.
The SyncFiles=true path is fine: updateFileInfoForLocalFiles already calls reconstructSeriesIngestLocations, which reads the manifest bytes off disk (they were downloaded alongside the doc) and rebuilds ingest_locations from them.
The gap is that on SyncFiles=false the manifest bytes are not on disk -- there is nothing to read. So the reconstruction needs a fetch, and it needs to work the same way DID's read path does (VH-Lab/DID-matlab#201 / VH-Lab/DID-python#88): through the DID customFileHandler contract.
What NDI-matlab does (the shape to port)
VH-Lab/NDI-matlab#988 (three commits: 1a6ec2fc, 414e8572, a4dfb076) taught updateFileInfoForRemoteFiles.m to:
- Accept an optional
customFileHandlername-value argument. - After the file_info rewrite, if any series in
series_infohasn_present > 0and emptyingest_locations, fetch each such series' manifest bytes into a scratch directory (through the handler when supplied, or a direct fetch minted throughndi.cloud.api.filesotherwise) and callreconstructSeriesIngestLocationswith that scratch directory. - The scratch directory is cleaned up on function exit. Manifest bytes do not land in the DID file cache here; a subsequent member
opentrips DID#201's lazy fetch, which is what populates the cache. Cache eviction on a later session is answered by the same re-fetch path. seriesNameis deliberately empty in the handler context -- a non-emptyseriesNamemarks a MEMBER fetch, wheresourcePathis the manifest andctx.uidis the member; the manifest fetch itself is the opposite (sourcePathnames the manifest,ctx.uidnames the same manifest).- A handler failure leaves the entry un-reconstructed so DID's guard fires on
add_docswith the document's own identity -- the right signal a partial download deserves.
Offline coverage is tests/+ndi/+unittest/+database/TestUpdateFileInfoForRemoteFilesShape.m (VH-Lab/NDI-matlab#987): a mock handler serves manifest bytes by uid, and the test walks a doc through DID's store/read cycle so ingest_locations gets stripped (the exact shape a cloud round trip returns), then reshapes through updateFileInfoForRemoteFiles with the mock and pins the rebuilt ingest_locations.
What to change on the Python side
src/ndi/cloud/filehandler.py:
- Add an arity-aware
_dispatch_custom_file_handler(handler, dest_path, source_path, context)helper mirroringdid.implementations.sqlitedb.SqliteDb._dispatch_custom_file_handler. - Add a
_fetch_manifest(dest_path, cloud_dataset_id, manifest_uid, document_id, series_name, custom_file_handler=None, client=None)helper. With a handler, dispatch through it withseriesName=""in the context. Without a handler, callfetch_cloud_filedirectly, matching what the read-side does when a manifest is asked for by uid. - Modify
updateFileInfoForRemoteFilesto take an optional keyword-onlycustom_file_handler=Noneand, after the file_info rewrite, when anyseries_infoentry hasn_present > 0and emptyingest_locations, fetch each qualifying series' manifest bytes to atempfile.TemporaryDirectoryand callreconstructSeriesIngestLocations(doc_props, str(tmp_dir), cloud_dataset_id)before the temp dir is cleaned up. A per-manifest fetch failure is caught and logged; the entry is left un-reconstructed so DID's guard fires on add.
tests/test_cloud_series_reconstruction_remote.py (new file, or extend tests/test_cloud_series_reconstruction.py):
- Ordinary reshape: every file_info location becomes
ndicloud,ingest=0,delete_original=0, uid survives, location isndic://<dataset>/<uid>. - A doc whose
ingest_locationsis already populated skips reconstruction (no fetch, no handler call). - A doc without
series_infoskips the whole block and still reshapesfile_info. - End-to-end offline: a mock handler serves manifest bytes by uid; take a doc, run it through DID's
add_docs/get_docscycle soingest_locationsgets stripped, reshape viaupdateFileInfoForRemoteFiles(..., custom_file_handler=mock), and pin the rebuiltingest_locations(one entry per present member,ndic://location,ingest=0, one manifest fetch total withseriesName="").
Bridge yamls:
- Bump
updateFileInfoForRemoteFiles'smatlab_last_sync_hashinsrc/ndi/cloud/ndi_matlab_python_bridge.yamlto the NDI-matlab main head that carries #988 (currentlyaea0c72c), and update the decision_log to record the port rather than the deferral. - Update
src/ndi/cloud/sync/ndi_matlab_python_bridge.yaml's downloadNdiDocuments entry if its decision_log calls out the gap.
Not in scope
A Python analog of NDI-matlab's FileSeriesRoundTripTest.testMembersSurviveADownloadFromTheCloudWithSyncFilesFalse -- the live-cloud round-trip test. Belongs in its own change, layered after this one; also flagged in the closing note on #300.
References
- 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)
- Cloud companion: SyncFiles=false round-trip in FileSeriesRoundTripTest (#986) VH-Lab/NDI-matlab#988 (the SyncFiles=false fix on the MATLAB side)
- Offline test for a cloud-only series manifest (SyncFiles=false shape) (#986) VH-Lab/NDI-matlab#987 (the offline mock-handler coverage)
- File series: accessors, remote ingest, an unlocatable-series guard, and one fetch per uid VH-Lab/DID-matlab#185 (the MembersNotLocatable guard this fix satisfies)
- localSeriesManifestPath cannot fetch a remote manifest — series reads fail on cloud-only downloads VH-Lab/DID-matlab#201, Fetch a remote-only series manifest through the custom_file_handler (#87) VH-Lab/DID-python#88 (the customFileHandler contract for a manifest fetch)
- Retire the manifest cloud-reference workaround (companion to VH-Lab/NDI-matlab#986) #300 (parent, closed; carved-out follow-up noted in its closing comment)
Activity