Skip to content

Downgrade outgoing metadata to server's schema version on upload - #1898

Open
yarikoptic wants to merge 9 commits into
masterfrom
enh-downgrade
Open

Downgrade outgoing metadata to server's schema version on upload#1898
yarikoptic wants to merge 9 commits into
masterfrom
enh-downgrade

Conversation

@yarikoptic

@yarikoptic yarikoptic commented Aug 4, 2026

Copy link
Copy Markdown
Member

Summary

When the DANDI Archive server reports an older schema_version than the one
the client's dandischema is generating (as happened with
dandi/dandi-schema#342 before it was released as
dandischema==0.14.0), uploads fail because the server rejects metadata
with unknown fields. This PR wires an on-the-fly downgrade into every
call site where a metadata dict leaves the client:

DandiAPIClient._maybe_downgrade_metadata(metadata) is a no-op unless
the server is strictly older and its schema version is in
dandischema.consts.ALLOWED_TARGET_SCHEMAS; otherwise it delegates to
dandischema.metadata.migrate(..., to_version=<server_ver>, skip_validation=True).
check_schema_version's warning now tells the user whether an automatic
downgrade will be attempted or whether the library cannot downgrade to
this version at all.

Also updates the dandischema constraint to
>= 0.12.0, != 0.13.0, < 0.15.0. 0.13.0 is excluded because it dropped
the SIMPLE_DOWNGRADES entries for releaseNotes and sameAs, so
uploads to older Archive servers silently regress (the fall-through
short-circuits and sends unchanged metadata that the server rejects).
0.14.0 restored them.

Tracks: dandi/dandi-schema#343 · Depends on: dandi/dandi-schema#342 (released as dandischema==0.14.0)

Downgrade dispatch (single interceptor)

Rather than sprinkle _maybe_downgrade_metadata(...) at every metadata-
sending call site, DandiAPIClient.request runs
_maybe_downgrade_request_metadata over the outgoing json body.
A body is considered to carry DANDI metadata iff it is a dict whose
"metadata" value is itself a dict with both schemaKey and
schemaVersion as strings — the invariant shape of every DANDI
metadata payload the archive accepts. Non-matching bodies pass
through unchanged.

Any future metadata-sending endpoint gets the downgrade for free —
no per-call-site plumbing required.

Behavior when downgrade can't strip a populated field

dandischema.metadata.migrate raises ValueError if a
post-server-version field (sameAs added 0.8.0, releaseNotes added
0.7.0) carries a non-empty value that can't be simply stripped. The
client catches this, logs a warning, and forwards the original
metadata so the server decides. Prior draft of this PR let the
ValueError propagate and crash the upload.

Graceful behavior against older installed dandischema
  • With dandischema==0.12.0 (client itself generates 0.7.0 metadata,
    matching what old servers speak) — _maybe_downgrade_metadata
    short-circuits: nothing to downgrade from. Same behavior as before
    this PR.
  • With dandischema==0.13.0 — the client generates 0.8.0 metadata but
    the library has no downgrade path. This combination breaks uploads
    to older Archive servers, which is why 0.13.0 is excluded from the
    version range.
  • With dandischema>=0.14.0 — full downgrade path is active.
  • The new tests that assert downgrade behavior are gated via a
    _needs_downgrade skip marker that requires both
    DANDI_SCHEMA_VERSION >= 0.8.0 and "0.7.0" in ALLOWED_TARGET_SCHEMAS,
    so py3-lowest (which resolves to dandischema==0.12.0) cleanly
    skips them while the default env exercises them fully.

Test plan

  • pytest -k "downgrade or check_schema_version or retry_logging"
    against dandischema==0.14.0 (default env) — 23 passed
  • Same, under tox -e py3-lowest with dandischema==0.12.0 — 15
    passed, 8 skipped cleanly via the tightened _needs_downgrade gate
  • tox -e py3-lowest -- dandi/tests/test_metadata.py dandi/tests/test_utils.py dandi/tests/test_files.py dandi/tests/test_helpers.py dandi/tests/test_organize.py dandi/validate/tests/ — 1023 passed,
    302 skipped, 16 xpassed
  • tox -e lint and tox -e typing clean
  • Full CI green
  • Reviewer: sanity-check _maybe_downgrade_metadata fall-through
    wording in the check_schema_version warning (msg_downgrade) —
    it now branches on whether the library supports downgrade to the
    target version

yarikoptic and others added 2 commits August 4, 2026 11:08
When the DANDI Archive server reports a `schema_version` older than the one
carried in metadata the client is about to send, `DandiAPIClient` now attempts
to downgrade the metadata via `dandischema.metadata.migrate(..., to_version=
server_schema_version, skip_validation=True)` before sending.  This unblocks
uploads to servers that trail the client's `dandischema` (dandi-schema #342,
tracking dandi-schema #343).

Additions:

- `DandiAPIClient.server_schema_version` — `@cached_property` reading
  `/info/` once; raises if the server does not expose `schema_version`.
- `DandiAPIClient._maybe_downgrade_metadata(metadata)` — no-op unless the
  server is strictly older *and* its version is in
  `dandischema.consts.ALLOWED_TARGET_SCHEMAS`; otherwise returns the
  metadata unchanged (graceful fall-through when the installed dandischema
  has no downgrade path for the target).

Call sites (every place a metadata dict leaves the client):

- `create_dandiset`
- `RemoteVersion.set_raw_metadata`
- `RemoteBlobAsset.set_raw_metadata`
- `RemoteZarrAsset.set_raw_metadata`
- `LocalFileAsset.iter_upload` (`dandi/files/bases.py`)
- `ZarrAsset.iter_upload` (`dandi/files/zarr.py`)

`check_schema_version` no longer just warns and shrugs when the server is
older: the warning now tells the user whether an automatic downgrade will be
attempted (target ∈ `ALLOWED_TARGET_SCHEMAS`) or whether the library cannot
downgrade to this version at all.

Tests (in `dandi/tests/test_dandiapi.py`, using the existing `responses`
pattern):

- `test__maybe_downgrade_metadata` — parametrized over
  same/one-minor-older/two-minor-older/unsupported-target server
  versions, asserting the correct `schemaVersion` and which of `sameAs`
  (0.8.0-added) / `releaseNotes` (0.7.0-added) get stripped vs kept.
- `test_set_raw_metadata_downgrades_on_older_server` — end-to-end reproducer
  of the dandi-schema #342 CI failure via a mocked 0.7.0 server; asserts
  the outgoing PUT body carries `schemaVersion=0.7.0` and no `sameAs`.

Verified locally that the patched client behaves correctly against both a
downgrade-capable dandischema (release-schema PR: full fix, all 16 tests
green) and prior released dandischema (0.13.0 / 0.12.1: graceful no-op,
no crash — same behavior as before this patch).

Co-Authored-By: Claude Code 2.1.221 / Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to previous commit "downgrade outgoing metadata to server's schema
version on upload".  Applies fixes surfaced by an independent code review.

Blockers:

- `tests/test_dandiapi.py`: move `from ..dandiapi import RemoteDandiset,
  VersionStatus` to module-top imports; drop the shadowing in-function
  `from datetime import datetime, timezone` (already imported at module
  top).  Complies with the project's "no function-local imports" rule.
- `dandiapi.py`: `_maybe_downgrade_metadata` now returns
  `cast(Dict[str, Any], migrate(...))` — dandischema has no type stubs,
  so mypy would otherwise flag `no-any-return` in strict mode.

Behavior fixes:

- `_maybe_downgrade_metadata`: catch `ValueError` from
  `dandischema.metadata.migrate` (raised when a post-server-version field
  such as `sameAs` or `releaseNotes` is populated and cannot be simply
  stripped), log a warning, and return the original metadata unchanged
  so the server can decide.  Prior behavior would crash the upload.
- Add two `test__maybe_downgrade_metadata_falls_through_on_populated_field`
  cases (populated `sameAs` on 0.7.0 server; populated `releaseNotes` on
  0.6.10 server) asserting fall-through + warning.

Refactor:

- Introduce `DandiAPIClient.server_info` (`@cached_property`) as the single
  source for the server's `/info/` response; `check_schema_version` and
  `server_schema_version` both use it now.  Removes the duplicate
  `/info/` fetch that the two paths previously did independently.
- Drop the redundant `obj_ver == server_ver_str` guard in
  `_maybe_downgrade_metadata` (subsumed by the `>=` version comparison).
- Drop the unused `mocker: MockerFixture` fixture from
  `test_set_raw_metadata_downgrades_on_older_server`.

Test infrastructure:

- Add a `_needs_downgrade` skip marker keyed on
  `"0.7.0" in dandischema.consts.ALLOWED_TARGET_SCHEMAS`; apply it to
  parametrize cases and tests that exercise downgrade-to-older-schema
  behavior.  Verified locally that the suite skips cleanly against
  released `dandischema==0.13.0` (which has no downgrade path to older
  schemas) and passes fully against the release-schema branch.

Dependency:

- `pyproject.toml`: widen `dandischema` to `>= 0.12.0, < 0.14.0`.  With
  the downgrade wiring in place — and its graceful fall-through when the
  installed dandischema lacks migration paths — the released 0.13.x line
  is safe to use as well.

Co-Authored-By: Claude Code 2.1.221 / Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.52632% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.01%. Comparing base (2b5f8ea) to head (85efc16).

Files with missing lines Patch % Lines
dandi/dandiapi.py 77.50% 9 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1898      +/-   ##
==========================================
+ Coverage   76.96%   77.01%   +0.05%     
==========================================
  Files          88       88              
  Lines       12882    12957      +75     
==========================================
+ Hits         9914     9979      +65     
- Misses       2968     2978      +10     
Flag Coverage Δ
unittests 77.01% <90.52%> (+0.05%) ⬆️

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

☔ 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.

* origin/master:
  deps: tighten 8 floors to match the versions `py3-lowest` actually resolves
  deps: add lower bounds to previously-unbounded direct dependencies
  ci: add py3-lowest tox env + lowest-deps CI mode

 Conflicts:
	pyproject.toml -- since here we relaxed dandischema depend for downgrades testing
Comment thread dandi/dandiapi.py Fixed
@yarikoptic yarikoptic added minor Increment the minor version when merged schema Issues relating to metadata schema labels Aug 5, 2026
yarikoptic and others added 6 commits August 5, 2026 23:58
Released dandischema 0.13.0 dropped the SIMPLE_DOWNGRADES entries for
`releaseNotes` (0.7.0) and `sameAs` (0.8.0), so `_maybe_downgrade_metadata`
falls through and uploads to older Archive servers fail with unknown-field
errors.  0.14.0 restored those migrations.

Widen the range and exclude just 0.13.0:

    dandischema >= 0.12.0, != 0.13.0, < 0.15.0

< 0.15.0 keeps the upper bound conservative; we can widen it further when
0.15.0 lands and is verified to preserve the downgrade path.

Co-Authored-By: Claude Code 2.1.221 / Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pydantic's mypy plugin materializes `__init__` from *field names*, so
`Version(version="draft", ...)` — which uses the `alias="version"`
declared on `identifier` — is a type error even though
`populate_by_name=True` on `APIBase` accepts it at runtime.  Use the
field name directly.

Fixes the typing job on the enh-downgrade branch.

Co-Authored-By: Claude Code 2.1.221 / Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`check_schema_version` was raising `SchemaVersionError` for the exact
scenario this branch exists to handle: server on 0.7.x, client on 0.8.x
(different 0.x minor, treated as "incompatible"), so no test that goes
through the sample-dandiset fixtures on Ubuntu (docker dandi-api) could
even reach `_maybe_downgrade_metadata`.

Consolidate the two "server older" branches: if dandischema knows a
downgrade path (`server_version in ALLOWED_TARGET_SCHEMAS`) *or* the
versions are within the same MAJOR.MINOR (0.x.y) / MAJOR (1.x.y)
compatibility band, just warn -- with the existing message about
whether a downgrade will be attempted or is unsupported.  Only raise
when neither condition holds.

Also mark the three new AI-generated downgrade tests with
`@pytest.mark.ai_generated` per repo convention, and extend
`test_check_schema_version` with two parametrize cases covering the
new behavior (0.7.0/0.8.0 and 0.6.10/0.8.0 both no longer raise).

Co-Authored-By: Claude Code 2.1.221 / Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ate var

CodeQL flagged `same_compat_band` as "may be used before initialized" on
`check_schema_version`.  It was in fact unconditionally assigned in the
same `elif` branch, but eliminating the intermediate variable and
inlining the condition side-steps the analyzer's confusion entirely.
Same semantics.

Co-Authored-By: Claude Code 2.1.221 / Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Instead of sprinkling `_maybe_downgrade_metadata(...)` at every metadata-
sending call site, override `DandiAPIClient.request` to run
`_maybe_downgrade_request_metadata` over the outgoing ``json`` body.
A body is considered to carry DANDI metadata iff it is a ``dict`` whose
``"metadata"`` value is itself a ``dict`` with both ``schemaKey`` and
``schemaVersion`` as strings — the invariant shape of every DANDI
metadata payload the archive accepts.  Non-matching bodies pass
through unchanged.

Drops the six spot-patches added in the previous commit:

- `DandiAPIClient.create_dandiset`
- `RemoteVersion.set_raw_metadata`
- `RemoteBlobAsset.set_raw_metadata`
- `RemoteZarrAsset.set_raw_metadata`
- `LocalFileAsset.iter_upload` (`dandi/files/bases.py`)
- `ZarrAsset.iter_upload` (`dandi/files/zarr.py`)

Any future metadata-sending endpoint now gets the downgrade for free.

Also adds two unit tests:

- `test__maybe_downgrade_request_metadata_shape_check` — verifies the
  shape check does not touch non-matching bodies (non-dict, missing
  `metadata`, `metadata` not a dict, missing/non-str `schemaKey` or
  `schemaVersion`, or `metadata` sitting under a different key).
- `test__maybe_downgrade_request_metadata_downgrades` — verifies the
  interceptor returns a new body (does not mutate input) when the
  shape matches, and correctly downgrades the nested metadata dict.

The end-to-end `test_set_raw_metadata_downgrades_on_older_server`
test is unchanged and now exercises the full
`set_raw_metadata` -> `client.put` -> `request` -> interceptor path.

Co-Authored-By: Claude Code 2.1.221 / Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The downgrade parametrizations feed the client a 0.8.0-schema
metadata dict and expect `dandischema.metadata.migrate` to bring it
down to 0.7.0.  Everything they need -- the 0.8.0-era metadata schema
plus `SIMPLE_DOWNGRADES` entries for `sameAs` and `releaseNotes` --
landed together in `dandischema==0.14.0`.

Older dandischema in the allowed range (0.12.0, reached only under
`py3-lowest`) generates 0.7.0 metadata and has no downgrade path, so
these tests are meaningless there and previously failed inside
`migrate()` because 0.12.0 doesn't recognize "0.8.0" as a valid input
at all.  0.13.0 is excluded from `pyproject.toml`.

Replace the compound two-condition gate (which spoke about
`DANDI_SCHEMA_VERSION` and `ALLOWED_TARGET_SCHEMAS` separately) with a
single direct check on the library version.  With this,
`py3-lowest`/0.12.0 cleanly skips the 8 downgrade parametrizations
while the default env (0.14.0+) runs the full 23.

Co-Authored-By: Claude Code 2.1.221 / Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

minor Increment the minor version when merged schema Issues relating to metadata schema

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants