Skip to content

Add ndi.cloud.api.files.waitForFileUpload; wait for upload processing in uploadSingleFile and FilesTest - #1027

Merged
stevevanhooser merged 3 commits into
mainfrom
claude/wait-for-file-upload
Oct 13, 2026
Merged

stevevanhooser merged 3 commits into
mainfrom
claude/wait-for-file-upload

Conversation

@stevevanhooser

Copy link
Copy Markdown
Contributor

Why

FilesTest/testSingleFileUploadAndDownloadUseCurl and ...UseCurlStop fail intermittently in Cloud CI. This happened on #979 in runs 38418516741 and 38461572210, and the first passed on re-run. In the failures, curl reports success, but listFiles/detail then show uploaded=false, size=0, crc32="" and no downloadUrl.

That is the record's pending state from before the server processes the upload, so the PUT itself succeeded:

  • putFiles(useCurl) already runs curl -fsSL, so a 4xx from S3 would have made b_put false.
  • The x-amz-checksum-crc32=AAAAAA== query parameter on no-checksum URLs is not new with ndi-cloud-node#162. ChecksumAlgorithm: 'CRC32' has been in the signer since April (ndi-cloud-node#47). These tests passed with it until now, including on main 1426fcd today. Spikesorter lisandro #162 records that S3 ignores a checksum sent in the query string.
  • After a PUT, an S3-triggered lambda (rawFileUploadHandler, reservedConcurrency: 8) records the upload. Files extracted from bulk uploads land in the same bucket and wake that same lambda. Lightsheet pyramids: ingest from OME-Zarr or SmartSPIM, chunk materialization, cloud round trip, and napari viewing #979 adds TestSignedUrlSetJobScaling and TestSignedUrlSetJobFixture, which bulk-upload about 600 files to prod per run, and the other Cloud CI shards run in parallel. When those overlap, this test's event can wait in line for more than the fixed pause(10).

The last point is inferred from the code and the run history. I did not have the CI logs or CloudWatch to confirm the timing overlap.

What

  • ndi.cloud.api.files.waitForFileUpload(datasetId, uids, ...) (new), with the implementation class ndi.cloud.api.implementation.files.WaitForFileUpload.
    • Polls lookupFiles with exponential backoff until every uid is marked uploaded or the timeout passes (default 120 s).
    • Takes several uids and only re-checks the ones still pending.
    • A failed lookup is retried, not treated as a failure.
    • Returns state, files, pending and elapsed.
    • Same layout as waitForAllBulkUploads, with a pollStatus method tests can override, as WaitForFileTierJob does.
  • ndi.cloud.uploadSingleFile gets a 'waitForUpload' option, default true.
    • Single-file uploads call waitForFileUpload after the PUT and fail if the server hasn't recorded the file in time.
    • Bulk uploads pass waitForCompletion to putFiles.
    • A bulk wait failure now reports the job state instead of failing on a missing .message field.
  • ndi.cloud.sync.internal.uploadFilesForDatasetDocuments (serial strategy) passes 'waitForUpload', false per file. It then waits for all the uids it sent with one waitForFileUpload call, so a large sync doesn't wait on the server after every file.
  • FilesTest:
    • The seven pause(10) calls after single-file uploads now call waitForFileUpload.
    • testUploadSingleFileRecordsChecksum now checks that the file is already marked uploaded when uploadSingleFile returns.

Tests

  • FileUploadWaitMockTest (offline, via the new ndi.test.helper.ScriptedFileUpload) covers:
    • a file that is pending and then uploaded (the CI failure)
    • a uid with no record yet
    • a timeout
    • a lookup error that is retried
    • several uids where some finish first (only pending ones are re-checked)
    • duplicate uids
  • FilesTest.testWaitForFileUpload (live) covers the happy path, checking the reported uid and size, and a 3 s timeout on a uid that was never uploaded.

Written without a local MATLAB runtime. Nothing here has been run, so it needs CI or a MATLAB run to validate.

🤖 Generated with Claude Code

https://claude.ai/code/session_016pWYt77CB6boPRzKxxu9e5


Generated by Claude Code

A single-file PUT returning 200 only means S3 has the object; the
server records the upload (uploaded, size, crc32) in an S3-triggered
lambda afterwards. That lambda runs with reservedConcurrency 8, so when
another Cloud CI shard is bulk-uploading (each extracted file wakes the
same lambda) the record can lag the PUT by more than the fixed 10 s
FilesTest waited, and the curl upload tests saw uploaded=false, size=0.

waitForFileUpload polls lookupFiles with exponential backoff until every
given uid is marked uploaded, or the timeout (default 120 s) elapses. It
is the single-file counterpart to waitForBulkUpload, with the same
call-class layout and a pollStatus seam like WaitForFileTierJob.

Tests:
- FileUploadWaitMockTest (offline, via ndi.test.helper.ScriptedFileUpload):
  pending-then-uploaded, missing record, timeout, transient lookup error,
  several uids, duplicate uids.
- FilesTest.testWaitForFileUpload (live): happy path and a timeout on a
  uid that was never uploaded.
- FilesTest's seven pause(10) waits after single-file uploads now call
  waitForFileUpload.

Authored without a local MATLAB runtime; needs MATLAB to validate/run.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016pWYt77CB6boPRzKxxu9e5
uploadSingleFile gains 'waitForUpload' (default true). For a single-file
upload it calls ndi.cloud.api.files.waitForFileUpload after the PUT and
fails if the server has not recorded the file within the timeout; for a
bulk upload it passes waitForCompletion to putFiles so the extraction
job is awaited. A bulk wait failure now reports the job state instead of
failing on a missing .message field.

The serial sync path (uploadFilesForDatasetDocuments) passes
'waitForUpload', false and waits for all sent uids with one
waitForFileUpload call at the end, so a large sync does not wait on the
server after every file.

FilesTest.testUploadSingleFileRecordsChecksum no longer waits itself:
it checks the file is already marked uploaded when uploadSingleFile
returns.

Authored without a local MATLAB runtime; needs MATLAB to validate/run.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016pWYt77CB6boPRzKxxu9e5
@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

1 439 tests  +7   1 406 ✅ +7   7m 38s ⏱️ +45s
  211 suites +1      33 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit c982a18. ± Comparison against base commit 1426fcd.

♻️ This comment has been updated with latest results.

The test class lives in package ndi.unittest, so the bare
FileUploadWaitMockTest.lookup(...) static-method calls failed with
MATLAB:undefinedVarOrClass in all 7 tests. Move lookup/scripted to
file-local functions (lookupResult/scriptedCall), which need no package
qualification.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016pWYt77CB6boPRzKxxu9e5
@codecov

codecov Bot commented Oct 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 49.41176% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 30.36%. Comparing base (1426fcd) to head (c982a18).

Files with missing lines Patch % Lines
src/ndi/+ndi/+cloud/uploadSingleFile.m 0.00% 18 Missing ⚠️
...rc/ndi/+ndi/+cloud/+api/+files/waitForFileUpload.m 0.00% 12 Missing ⚠️
...d/+sync/+internal/uploadFilesForDatasetDocuments.m 0.00% 11 Missing ⚠️
...ud/+api/+implementation/+files/WaitForFileUpload.m 95.45% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1027      +/-   ##
==========================================
+ Coverage   30.32%   30.36%   +0.03%     
==========================================
  Files         876      878       +2     
  Lines       44217    44299      +82     
==========================================
+ Hits        13410    13452      +42     
- Misses      30807    30847      +40     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@stevevanhooser
stevevanhooser merged commit 44e707a into main Oct 13, 2026
12 checks passed
@stevevanhooser
stevevanhooser deleted the claude/wait-for-file-upload branch October 13, 2026 23:42
stevevanhooser pushed a commit that referenced this pull request Oct 13, 2026
Brings in #1027 (waitForFileUpload; uploadSingleFile
waits for NDI Cloud to record the upload; FilesTest uses it).

Conflict in uploadFilesForDatasetDocuments: kept both sides -- this
branch's files_uploaded_count / Verbose summary and main's sentUids
collection with one waitForFileUpload after the serial loop.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
stevevanhooser pushed a commit to Waltham-Data-Science/NDI-python that referenced this pull request Oct 13, 2026
VH-Lab/NDI-matlab#1027 added +ndi/+cloud/+api/+files/waitForFileUpload.m
on main, and bridge completeness (which reads NDI-matlab main) failed
for want of an entry. Recorded as not_yet_ported with the reason.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
stevevanhooser pushed a commit to Waltham-Data-Science/NDI-python that referenced this pull request Oct 13, 2026
…inst NDI-matlab 9c21eb61

VH-Lab/NDI-matlab#1027 made both wait for NDI Cloud to record uploads
(through the new waitForFileUpload). Bumped both hashes with a note
that the wait is not ported yet, and gave the waitForFileUpload entry
its sync hash (eb583289).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wj6cuvAwgAaYN5dNM1o6L
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.

3 participants