Add ability to downgrade schema all the way to 0.6.10

[yarikoptic]

Reminder to myself: we run checks on no changes to schema only on PRs with "release" tag. Thus we did not run checks when we actually adjusted the schema in

• add releaseNotes property to published dandiset #185 by @bendichter.

Then I rushed merging #341
as I was not expecting surprises from adjusting labels (forgot about above).
The moral again, do not rush merging - let CI finish! ;)

I think the motivation/aspects are best captured in

• Provide "downgrade" functionality to migrate and make dandi-cli use migrate! #343
  but overall -- to facilitate upload to "older" server which might not be yet aware of a new(er) schema.

Boosting "patch" for both schema and library since just a "minor addition of a new feature" (mostly to schema).

TODOs

• release dandi-cli with Allow for client to provide records with newer COMPATIBLE or older (upgradable) version of schema to the server dandi-cli#1732
• lower boost here into .patch which would also trigger retesting,
• full CI, dandi-cli included, must come green

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.84%. Comparing base (8dff37f) to head (c1d0661).

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #342   +/-   ##
=======================================
  Coverage   97.84%   97.84%           
=======================================
  Files          16       16           
  Lines        2042     2042           
=======================================
  Hits         1998     1998           
  Misses         44       44           

Flag	Coverage Δ
unittests	97.84% <100.00%> (ø)

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

☔ View full report in Codecov by Sentry.
📢 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.

[yarikoptic]

@waxlamp here we are to get into that unpleasant situation to need to coordinate releasing since otherwise if client has this release more recent dandi-schema installed (possible with or without us pinning in client): we would get

ERROR tests/test_upload.py::test_upload_sync[False-False] - dandi.exceptions.SchemaVersionError: Server requires schema version 0.6.10; client only supports 0.7.0.  You may need to upgrade dandi and/or dandischema.

so the question(s)

• do you see any problem with us aiming to update versioned depends/release dandi-archive ATM?
• should we provide PR or you will?

[waxlamp]

@waxlamp here we are to get into that unpleasant situation to need to coordinate releasing since otherwise if client has this release more recent dandi-schema installed (possible with or without us pinning in client): we would get

ERROR tests/test_upload.py::test_upload_sync[False-False] - dandi.exceptions.SchemaVersionError: Server requires schema version 0.6.10; client only supports 0.7.0.  You may need to upgrade dandi and/or dandischema.

How could this happen if you are pinning dandischema in the client (other than a user going out of their way to forcibly upgrade dandischema in their virtual environment after installing dandi-cli)?

(For the record, it seems that the CLI does indeed pin dandischema to a minor version.)

so the question(s)

• do you see any problem with us aiming to update versioned depends/release dandi-archive ATM?

No problem, given that both the archive and the CLI are pinning to a specific version of dandischema already.

This does mean we need to coordinate releasing versions of both that do use the new version, but I'm not sure how we can avoid that without solving the bigger problem of enabling automatic migration between schema versions.

• should we provide PR or you will?

I imagine it would be part of dandi/dandi-archive#2606 (see this comment).

[yarikoptic]

@waxlamp here we are to get into that unpleasant situation to need to coordinate releasing since otherwise if client has this release more recent dandi-schema installed (possible with or without us pinning in client): we would get

ERROR tests/test_upload.py::test_upload_sync[False-False] - dandi.exceptions.SchemaVersionError: Server requires schema version 0.6.10; client only supports 0.7.0.  You may need to upgrade dandi and/or dandischema.

How could this happen if you are pinning dandischema in the client (other than a user going out of their way to forcibly upgrade dandischema in their virtual environment after installing dandi-cli)?

Simply because we are testing against these changes, which will give new version 0.7.0 -- that's the whole point. And it just reveals fragility of our tight binding of versions here and there and that pinning just a workaround for "user deployment" while developers, who need to test components where one is ahead of release, would get out of sync! What is critical here is that actually this new version would be just fine for dandi-cli... so I might reconsider and

• relax in dandi-cli to require the same MAJOR.MINOR only, not MAJOR.MINOR.PATCH (potentially with some hardcoded minimal we need since we were not consistent all the time before in 0.6 series)
• make this to release 0.6.11 not 0.7.0 schema, so we absorb within MAJOR.MINOR even backward compatible enhancements . If we need server to demand newer client (e.g. we know client extracts new important metadata), we can demand via /server-info but IMHO there is no way to demand matching schema version

then this would

• pass testing as long as we do not introduce backward compatible changes! and that is where cli might need to account for them

This does mean we need to coordinate releasing versions of both that do use the new version, but I'm not sure how we can avoid that without solving the bigger problem of enabling automatic migration between schema versions.

well, I think there is overall misunderstanding as we actually/apparently do not auto migrate metadata at all ATM! Server does not really care much, and client started to demand exact match to avoid missing recently added new metadata to be extracted/uploaded. Since here we talk about releaseNotes which client is not concerned about, it should not be concerned about being 'outdated' from client's perspective, so no boost of minimal client version is needed.

• should we provide PR or you will?

I imagine it would be part of dandi/dandi-archive#2606 (see this comment).

thanks for the pointers! Indeed, would make sense to boost in that PR to this new version simply to allow for this new feature (since you hard-pin).

[yarikoptic]

@waxlamp here is my solution on dandi-cli side which I am planing to merge/release to largely decouple back client and server in terms of dandi-schema:

• Allow for client to provide records with newer COMPATIBLE or older (upgradable) version of schema to the server dandi-cli#1732

please chime in! With that PR, if I revert here to upgrade within 0.6.x -- dandi-cli tests must not be affected! for 0.7.x indeed would keep failing ATM but in principle we could relax there too ;-)

[candleindark]

Left a comment. Will provide more.

[candleindark]

Left a few more comments.

[candleindark]

This migration function seems to expect the metadata instance to be a Dandiset instance.

[candleindark]

Validating PublishedDandiset instance may create a problem if #266 is put in. However, I guess we can have it specifically validate against the PublishDandiset schema if a PublishedDandiset instance is provided.

[candleindark]

@yarikoptic The CI tests are not triggered correctly. We are not even supposed to be testing against Python 3.9 at this point. I don't think there is anything wrong with the workflow yaml files. There is possible some problem with some settings in the Settings panel in this repo which I don't have access to. @bendichter may be able to provide some hint. I think he helped us solved a similar issue before.

[candleindark]

@yarikoptic Can we have an updated top post regarding this PR #342 (comment)? I want the see are the eventual intended change and what are decided eventually. The booting of DANDI_SCHEMA_VERSION version as a minor bump is no longer accurate, for example.

[satra]

@yarikoptic - is there an associated issue that this PR is responding to? i can imagine from the the title, but the description doesn't tell me the goal/use-case of the PR.

[yarikoptic]

@satra probably the best relevant is

• Provide "downgrade" functionality to migrate and make dandi-cli use migrate! #343

@candleindark I updated description and title a little. Hopefully makes it clearer. I will rebase and push and see if testing "coverage" changes/gets unstuck.

[yarikoptic]

FYI: I adjusted branch protection rules now and 3.9 can RiP !

[yarikoptic]

rright, not ready to release

ERROR tests/test_upload.py::test_upload_nonzarr_to_zarr_path - dandi.exceptions.SchemaVersionError: Server uses older incompatible schema version 0.7.0; client supports 0.8.0.

as client then should introduce use of this downgrading functionality... I guess we might need to go for a patch jump in schema version which should make it "legit" in the eye of the client which has a more relaxed logic now.

[yarikoptic]

FAILED tests/test_download.py::test_download_newest_version - ValueError: Dandiset 000077 is Valid: {
    "asset_validation_errors": [
        {
            "field": "",
            "message": "Metadata version 0.7.1 is not allowed. Allowed are: 0.4.4, 0.5.1, 0.5.2, 0.6.0, 0.6.1, 0.6.2, 0.6.3, 0.6.4, 0.6.5, 0.6.6, 0.6.7, 0.6.8, 0.6.9, 0.6.10, 0.7.0.",
            "path": "file.txt"
        },
        {
            "field": "",
            "message": "Metadata version 0.7.1 is not allowed. Allowed are: 0.4.4, 0.5.1, 0.5.2, 0.6.0, 0.6.1, 0.6.2, 0.6.3, 0.6.4, 0.6.5, 0.6.6, 0.6.7, 0.6.8, 0.6.9, 0.6.10, 0.7.0.",
            "path": "subdir1/apple.txt"
        },
        {
            "field": "",
            "message": "Metadata version 0.7.1 is not allowed. Allowed are: 0.4.4, 0.5.1, 0.5.2, 0.6.0, 0.6.1, 0.6.2, 0.6.3, 0.6.4, 0.6.5, 0.6.6, 0.6.7, 0.6.8, 0.6.9, 0.6.10, 0.7.0.",
            "path": "subdir2/banana.txt"
        },
        {
            "field": "",
            "message": "Metadata version 0.7.1 is not allowed. Allowed are: 0.4.4, 0.5.1, 0.5.2, 0.6.0, 0.6.1, 0.6.2, 0.6.3, 0.6.4, 0.6.5, 0.6.6, 0.6.7, 0.6.8, 0.6.9, 0.6.10, 0.7.0.",
            "path": "subdir2/coconut.txt"
        }
    ],

I would need to re-review since I thought that client is ok with upload of "compatible" one and here we should validate just fine... later