-
Notifications
You must be signed in to change notification settings - Fork 33
Allow for client to provide records with newer COMPATIBLE or older (upgradable) version of schema to the server #1732
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
Conversation
Server might need then to either wait until upgrade it becomes capable of
validating them or we might in the future to allow client to downgrade them (on
client; loselessly only) since server cannot yet handle them.
This behavior would facilitate testing of new releases of dandi-schema which now
would fail since we require 1-to-1 correspondence. See e.g.
dandi/dandi-schema#342 (comment)
00efeb2 to
2926ba0
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1732 +/- ##
==========================================
- Coverage 75.11% 75.08% -0.04%
==========================================
Files 84 84
Lines 11856 11862 +6
==========================================
Hits 8906 8906
- Misses 2950 2956 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
with such dirty script#!/bin/bash
export PS4='> '
set -x
set -eu
cd "$(mktemp -d ${TMPDIR:-/tmp}/dl-XXXXXXX)"
dandiset=000029
echo "Working at $PWD"
echo -ne "Current server info: "
curl --silent -X 'GET' \
'https://api.dandiarchive.org/api/info/' \
-H 'accept: application/json' | jq .
# exploring either dandi-archive cares about versions of metadata at all!
asset=356b20b7-ae80-4d42-9715-075492eb025d
name=orig
# get info since that is where blob_id is more readily accessible as well as metadata
curl --silent -X 'GET' \
"https://api.dandiarchive.org/api/dandisets/$dandiset/versions/draft/assets/$asset/info/" \
-H 'accept: application/json' > $name-info.json
blob_id=$(jq .blob -r $name-info.json)
echo -ne "Asset metadata schema: "
jq .metadata.schemaVersion $name-info.json
echo -ne "Validation status: "
curl --silent -X 'GET' \
"https://api.dandiarchive.org/api/dandisets/$dandiset/versions/draft/assets/$asset/validation/" \
-H 'accept: application/json'
# Let's reupload into another file with some antiquated version of schema under a new path:
jq .metadata $name-info.json \
| jq '.schemaVersion = "0.1.0"' \
| sed -e 's,\(path".*\)\.nwb,\1_dup-1.nwb,g' \
> $name-newass-meta.json
# we need to specify path as key?!
new_path=$(jq -r .path $name-newass-meta.json)
echo -e "New path arg: $new_path"
cat $name-newass-meta.json \
| jq --arg blob_id "$blob_id" --arg path "$new_path" --slurpfile metadata $name-newass-meta.json \
'{ blob_id: $blob_id, path: $path, metadata: $metadata[0] }' \
> $name-newass-put-data.json
curl -X 'PUT' \
"https://api.dandiarchive.org/api/dandisets/$dandiset/versions/draft/assets/$asset/"\
-H 'accept: application/json' \
-H 'Content-Type: application/json' \
-H "Authorization: token $DANDI_API_KEY" \
-d "@$name-newass-put-data.json" > $name-newass-put-result.json
new_asset=$(jq .asset_id $name-newass-put-result.json)
# is it valid???
curl --silent -X 'GET' \
"https://api.dandiarchive.org/api/dandisets/$dandiset/versions/draft/assets/$new_asset/validation/" \
-H 'accept: application/json'
I discovered
so, I think , Ideally it should be a server which exposes that information since it is concerned about its behavior and it in this case where server operates with newer version of the library and client tries to upload with older! edit: I verified that if I specify version within that set, I did get new asset validate just fine -- see this version of asset with schema 0.6.1 which is reported valid |
candleindark
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let a few comments
…upgradable ATM server DOES NOT CARE! So we already allow for assets being created by other clients with outdated schema etc. They fail validation unless could be upgraded! Changes might need to be adjusted based on the name for the /info record field as would be decided in dandi/dandi-archive#2624 or PR to solve it
Co-authored-by: Isaac To <candleindark@users.noreply.github.com>
7c1bd6d to
9e2f852
Compare
Running tests with --log-cli-level=WARNING shows:
dandi/tests/test_dandiapi.py::test_check_schema_version[1.0.0-1.2.3-False-None]
------------------------------------------------------------------ live log call -------------------------------------------------------------------
WARNING dandi:dandiapi.py:710 Server uses schema version 1.0.0 older than client's 1.2.3 (dandischema library 0.11.1). Server might fail to validate such assets and you might not be able to publish this dandiset until server is upgraded. Alternatively, you may downgrade dandischema and reupload.
PASSED
dandi/tests/test_dandiapi.py::test_check_schema_version[1.6.7-1.7.0-False-None]
------------------------------------------------------------------ live log call -------------------------------------------------------------------
WARNING dandi:dandiapi.py:710 Server uses schema version 1.6.7 older than client's 1.7.0 (dandischema library 0.11.1). Server might fail to validate such assets and you might not be able to publish this dandiset until server is upgraded. Alternatively, you may downgrade dandischema and reupload.
PASSED
dandi/tests/test_dandiapi.py::test_check_schema_version[1.6.7-1.8.3-False-None]
------------------------------------------------------------------ live log call -------------------------------------------------------------------
WARNING dandi:dandiapi.py:710 Server uses schema version 1.6.7 older than client's 1.8.3 (dandischema library 0.11.1). Server might fail to validate such assets and you might not be able to publish this dandiset until server is upgraded. Alternatively, you may downgrade dandischema and reupload.
PASSED
dandi/tests/test_dandiapi.py::test_check_schema_version[1.6.7-2.7.0-True-Server uses older incompatible schema version 1.6.7; client supports 2.7.0] PASSED
I think it is informative enough!
|
Ok, let's begin this week with this release ;-) |
|
🚀 PR was released in |
Summary over compatibility is in the docstring for the function in the diff. Citing current one here for shortcut:
notes:
- the same;is the current behavior which causes us troubles.migrateand make dandi-cli usemigrate! dandi-schema#343And yes , it is looking into the future as for us to establish sensible behavior(s), and we might need to adjust depending on how we address
but overall point is that dandi-cli was too protective of dandi-archive which does not really care what version of metadata you feed it with. Since there is other clients etc, if we want to restrict anyhow more -- we need to do it on server side! Meanwhile -- I am relaxing very sensibly requirements on client side! This way we very much decouple back our client from server and ease migrations etc.
Extra feature could be downgrades of schema records (dandi/dandi-schema#343) but that is yet to be decided on.
Additional refs:
Here I would introduce notion of "compatible" schema and forbid upload from "incompatible".
TODO