claude produced out of experience in
Summary
dandi upload reports uninformative and slightly misleading STATUS /
MESSAGE values for Zarr assets. Raised by @kabilar during review of
#1816 (see comment).
Observed behavior
1. Unchanged Zarr is reported as done / exists - reuploading
A dandiset whose local Zarr is bit-identical to the remote Zarr renders
as:
PATH SIZE ... STATUS MESSAGE
...atives/dandi-cli-1816-test/HG9_Z1_Y49.nii.zarr 5.9 GB ... done exists - reuploading
...where the same-hash short-circuit for a regular blob would render
skipped / file exists.
2. Modified Zarr uses the same non-descriptive message
After a local edit, the same asset renders done / exists - reuploading
with no indication of what changed — no distinction between additions,
deletions, and modifications, and no idea of the affected size (which
matters because the SIZE column reflects the whole-Zarr size, not the
delta).
Root cause
check_replace_asset() in dandi/upload.py:547-548 unconditionally
short-circuits Zarr assets:
if isinstance(local_asset, ZarrAsset):
return (True, {"message": "exists - reuploading"})
It never compares local vs remote, and never differentiates additions
from modifications from deletions. In contrast, the non-Zarr path at
dandi/upload.py:579-583 does perform an etag/mtime comparison and
returns skip_file("file exists") (i.e. STATUS=skipped) when the
local and remote copies match.
Proposed changes
Change 1 -- Detect unchanged Zarr and skip
For a ZarrAsset where the local tree matches the remote Zarr (e.g. by
comparing per-entry digests, or by comparing the aggregated Zarr
checksum), return skip_file("file exists") so that STATUS=skipped / MESSAGE=file exists, matching the non-Zarr case.
Note that the diff needed to decide "unchanged" is currently only
computed inside ZarrAsset.iter_upload() (dandi/files/zarr.py) after
Zarr registration. Two implementation choices:
(a) Do a cheap upfront comparison in check_replace_asset (e.g. via
per-entry digest listing) at the cost of an extra API round-trip
per asset.
(b) Let iter_upload yield an early skipped status once it has
computed the diff, and have the caller replace the initial
exists - reuploading marker. This avoids the upfront cost
but leaks knowledge about Zarr internals into the reporter.
Change 2 -- Describe what is changing
When a Zarr is changing, replace exists - reuploading with the
kind and size of the modification. @kabilar's proposed vocabulary:
file exists - uploading additional objects (N GB)
file exists - deleting objects (N GB)
file exists - modifying objects (N GB)
The relevant sizes are already tracked inside
ZarrAsset.iter_upload() via to_upload.total_size and the
per-entry sizes of to_delete
(dandi/files/zarr.py:655,728); they just aren't surfaced as
MESSAGE. This overlaps with implementation choice (b) above.
Interaction with --zarr-mode patch (from #1816)
Once #1816 lands, patch mode does not delete remote-only entries, so
"deleting objects" should not appear for --zarr-mode patch. The
choice of message per mode:
| Mode |
Local == remote |
Local adds only |
Local modifies |
Local also drops entries |
full (dflt) |
skipped |
- uploading … (N GB) |
- modifying … (…) |
- deleting … (…) |
patch |
skipped |
- uploading … (N GB) |
- modifying … (…) |
not applicable |
Scope
Orthogonal to #1816 -- misreport exists on master today. Filing
separately so #1816 can land on its current scope and this can be
picked up as a UX follow-up.
claude produced out of experience in
Summary
dandi uploadreports uninformative and slightly misleadingSTATUS/MESSAGEvalues for Zarr assets. Raised by @kabilar during review of#1816 (see comment).
Observed behavior
1. Unchanged Zarr is reported as
done / exists - reuploadingA dandiset whose local Zarr is bit-identical to the remote Zarr renders
as:
...where the same-hash short-circuit for a regular blob would render
skipped / file exists.2. Modified Zarr uses the same non-descriptive message
After a local edit, the same asset renders
done / exists - reuploadingwith no indication of what changed — no distinction between additions,
deletions, and modifications, and no idea of the affected size (which
matters because the
SIZEcolumn reflects the whole-Zarr size, not thedelta).
Root cause
check_replace_asset()indandi/upload.py:547-548unconditionallyshort-circuits Zarr assets:
It never compares local vs remote, and never differentiates additions
from modifications from deletions. In contrast, the non-Zarr path at
dandi/upload.py:579-583does perform an etag/mtime comparison andreturns
skip_file("file exists")(i.e.STATUS=skipped) when thelocal and remote copies match.
Proposed changes
Change 1 -- Detect unchanged Zarr and skip
For a
ZarrAssetwhere the local tree matches the remote Zarr (e.g. bycomparing per-entry digests, or by comparing the aggregated Zarr
checksum), return
skip_file("file exists")so thatSTATUS=skipped / MESSAGE=file exists, matching the non-Zarr case.Note that the diff needed to decide "unchanged" is currently only
computed inside
ZarrAsset.iter_upload()(dandi/files/zarr.py) afterZarr registration. Two implementation choices:
(a) Do a cheap upfront comparison in
check_replace_asset(e.g. viaper-entry digest listing) at the cost of an extra API round-trip
per asset.
(b) Let
iter_uploadyield an earlyskippedstatus once it hascomputed the diff, and have the caller replace the initial
exists - reuploadingmarker. This avoids the upfront costbut leaks knowledge about Zarr internals into the reporter.
Change 2 -- Describe what is changing
When a Zarr is changing, replace
exists - reuploadingwith thekind and size of the modification. @kabilar's proposed vocabulary:
file exists - uploading additional objects (N GB)file exists - deleting objects (N GB)file exists - modifying objects (N GB)The relevant sizes are already tracked inside
ZarrAsset.iter_upload()viato_upload.total_sizeand theper-entry sizes of
to_delete(
dandi/files/zarr.py:655,728); they just aren't surfaced asMESSAGE. This overlaps with implementation choice (b) above.Interaction with
--zarr-mode patch(from #1816)Once #1816 lands, patch mode does not delete remote-only entries, so
"deleting objects" should not appear for
--zarr-mode patch. Thechoice of message per mode:
full(dflt)skipped- uploading … (N GB)- modifying … (…)- deleting … (…)patchskipped- uploading … (N GB)- modifying … (…)Scope
Orthogonal to #1816 -- misreport exists on
mastertoday. Filingseparately so #1816 can land on its current scope and this can be
picked up as a UX follow-up.