Skip to content

dandi upload: STATUS/MESSAGE for Zarr assets is uninformative #1893

Open

Description

@yarikoptic

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions