Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start Zarr download as soon as first page of entries is obtained #1569

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

jwodder
Copy link
Member

@jwodder jwodder commented Jan 28, 2025

Closes #1447.

@jwodder jwodder added UX patch Increment the patch version when merged cmd-download zarr labels Jan 28, 2025
@jwodder jwodder requested a review from yarikoptic January 28, 2025 19:59
Copy link

codecov bot commented Jan 28, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.43%. Comparing base (5c5c3a3) to head (2db4dba).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1569      +/-   ##
==========================================
+ Coverage   88.35%   88.43%   +0.07%     
==========================================
  Files          78       78              
  Lines       10735    10735              
==========================================
+ Hits         9485     9493       +8     
+ Misses       1250     1242       -8     
Flag Coverage Δ
unittests 88.43% <100.00%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@yarikoptic
Copy link
Member

cool! Indeed starts to show progress right away! one gotcha only is that

image

size is reported 0 and that seems to be due to initial msg somewhere:

❯ DANDI_DEVEL=1 dandi download -f debug dandi://dandi/000026/sub-I48/ses-SPIM/micr/sub-I48_ses-SPIM_sample-BrocaAreaS03_stain-Somatostatin_SPIM.ome.zarr
2025-01-28 16:07:09,518 [ WARNING] Parallel downloads are not yet implemented for non-pyout format='debug'. Download will proceed serially.
{'size': 0, 'path': 'sub-I48_ses-SPIM_sample-BrocaAreaS03_stain-Somatostatin_SPIM.ome.zarr'}
{'status': 'downloading', 'path': 'sub-I48_ses-SPIM_sample-BrocaAreaS03_stain-Somatostatin_SPIM.ome.zarr'}
{'done': 0, 'done%': 0, 'path': 'sub-I48_ses-SPIM_sample-BrocaAreaS03_stain-Somatostatin_SPIM.ome.zarr'}
2025-01-28 16:07:11,989 [    INFO] File .zgroup in Zarr 7933bc72-ec3f-4043-8076-d7aa8a70e4f7 successfully downloaded
{'done': 24, 'done%': 0, 'path': 'sub-I48_ses-SPIM_sample-BrocaAreaS03_stain-Somatostatin_SPIM.ome.zarr'}
{'message': '1 done', 'path': 'sub-I48_ses-SPIM_sample-BrocaAreaS03_stain-Somatostatin_SPIM.ome.zarr'}

do you see where that size is reported, and whether could be set to e.g. None and not reported until determined?

@jwodder
Copy link
Member Author

jwodder commented Jan 28, 2025

@yarikoptic That's because the API reports that asset as having a size of zero:

$ curl -fsSL https://api.dandiarchive.org/api/dandisets/000026/versions/draft/assets/'?path=sub-I48/ses-SPIM/micr/sub-I48_ses-SPIM_sample-BrocaAreaS03_stain-Somatostatin_SPIM.ome.zarr' | jq .
{
  "count": 1,
  "next": null,
  "previous": null,
  "results": [
    {
      "asset_id": "60db4111-6d6e-4ad1-a161-63a2789cde2e",
      "blob": null,
      "zarr": "7933bc72-ec3f-4043-8076-d7aa8a70e4f7",
      "path": "sub-I48/ses-SPIM/micr/sub-I48_ses-SPIM_sample-BrocaAreaS03_stain-Somatostatin_SPIM.ome.zarr",
      "size": 0,
      "created": "2024-06-14T11:22:58.647070Z",
      "modified": "2024-06-14T11:22:58.663144Z"
    }
  ]
}

@yarikoptic
Copy link
Member

Ah, I found the worst exactly to try on in my bag history. Thanks for digging!

@yarikoptic yarikoptic merged commit 05f5691 into master Jan 28, 2025
26 checks passed
@yarikoptic yarikoptic deleted the gh-1447 branch January 28, 2025 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmd-download patch Increment the patch version when merged UX zarr
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make zarr download begin as soon as initial page of zarr files is obtained
2 participants