Set uploaded asset metadata schemaKey to "Asset"#1886
Open
candleindark wants to merge 3 commits into
Open
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1886 +/- ##
==========================================
+ Coverage 76.88% 76.89% +0.01%
==========================================
Files 88 88
Lines 12901 12909 +8
==========================================
+ Hits 9919 9927 +8
Misses 2982 2982
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Whatever the metadata was gathered as (e.g. a BareAsset), what is created on the server is an Asset. The server stores schemaKey verbatim and does not normalize it, so set it at the point of upload rather than relying on the caller. This matters because dandi-schema is changing BareAsset.schemaKey from "Asset" to "BareAsset". Metadata gathered via a BareAsset then serializes with schemaKey="BareAsset", so an asset created or updated from it would be stored with the wrong schemaKey. Pin it at every asset-endpoint write: iter_upload (LocalFileAsset, ZarrAsset) and set_raw_metadata (RemoteBlobAsset, RemoteZarrAsset); the latter is reached by the reextract command without going through iter_upload. A no-op against the current dandi-schema release (where BareAsset.schemaKey is still "Asset"), so it can ship first. Co-Authored-By: Claude Code 2.1.202 / Claude Opus 4.8 claude-opus-4-8 <noreply@anthropic.com>
c47044b to
f8b92a7
Compare
This was referenced Jul 7, 2026
Draft
`test_nwb2asset` and `test_nwb2asset_remote_asset` compared the `nwb2asset()` result against a `BareAsset` built with an explicit `schemaKey="Asset"`, which just restates the field default. `model_construct()` fills defaults in, so dropping it is a no-op against the current dandischema and ensures the equalities continue to hold once dandi-schema#419 lands, where `BareAsset.schemaKey` becomes "BareAsset".
… value `prepare_metadata()` returns a `BareAsset`, whose top-level `schemaKey` is `"Asset"` with the current dandischema but `"BareAsset"` under dandi/dandi-schema#419 (which pins `BareAsset.schemaKey` to its class name). To keep this test passing against either dandischema version, the expected `schemaKey` is set to the default of the `schemaKey` field in `BareAsset` before the comparison, and the four `metadata2asset*.json` files now store a placeholder for the field. The later `validate()` call checks the data against the `Asset` class, so `data_as_dict` is promoted to an Asset (`schemaKey = "Asset"`) beforehand.
Member
Author
|
@yarikoptic Please take a look at this PR. We will need this merged and released in order to have the integration tests in dandi/dandi-schema#419 to become green. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Pin
schemaKeyto"Asset"on every metadata dict written to an asset endpoint. This is the lockstep client change for dandi/dandi-schema#419, which changesBareAsset.schemaKeyfrom"Asset"to"BareAsset".Whatever the metadata was gathered as (e.g. a
BareAsset), what is created on the server is anAsset. The archive storesschemaKeyverbatim and does not normalize it, so onceBareAsset.schemaKeybecomes"BareAsset",BareAsset-derived metadata would be stored with the wrongschemaKey. Setting it at the point of upload keeps this correct regardless of how the metadata was gathered.It is a no-op against the current dandi-schema release (where
BareAsset.schemaKeyis still"Asset"), so it can merge and release ahead of the dandi-schema change.What changed
schemaKeyis set to"Asset"at every sink that writes to an asset create/update endpoint:LocalFileAsset.iter_uploadandZarrAsset.iter_upload(POST create / PUT replace), next to the existingmetadata.setdefault("path", ...)step. Covers the normal CLI upload, the deprecatedupload_raw_asset/iter_upload_raw_asset, and directLocalAsset.upload()/iter_upload()API use.RemoteBlobAsset.set_raw_metadataandRemoteZarrAsset.set_raw_metadata(PUT update). Thereextractservice command reaches these withBareAsset-derived metadata (nwb2assetreturns aBareAsset) without going throughiter_upload.RemoteDandisetVersion.set_raw_metadatais left untouched: it writes Dandiset metadata, not an asset.Test adaptations for the
BareAsset.schemaKeychangeTwo metadata tests hard-coded the top-level
schemaKeyof aBareAssetas"Asset", which becomes"BareAsset"under dandi/dandi-schema#419. Both now track the default of theschemaKeyfield inBareAssetrather than a literal, so they pass against the current dandischema and #419 alike:test_nwb2asset/test_nwb2asset_remote_asset: drop the redundant explicitschemaKey="Asset", which merely restated the field default.test_prepare_metadata: set the expectedschemaKeyto the default of theschemaKeyfield inBareAssetbefore the comparison (the fourmetadata2asset*.jsonfiles now store a placeholder for it), and promote the object to anAssetbefore thevalidate()call that checks it against theAssetclass.Test plan
black --checkclean on the changed files (verified out-of-band; the repo's pinned black currently crashes under Python 3.14, unrelated to this change)test_upload_downloadextended to assert the uploaded asset's storedschemaKey == "Asset"(needs the Docker-backed archive fixtures in CI)BareAsset.schemaKey == "BareAsset") via DO NOT MERGE: check #1886 schemaKey handling against dandi-schema #419 #1888, which repoints thedandischemadependency at the Consolidate Asset/Dandiset models; gate publish validation on datePublished dandi-schema#419 branch🤖 Generated with Claude Code