Skip to content
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

Pin dandischema to require the latest schema version #1570

Merged
merged 3 commits into from
Jan 30, 2025

Conversation

kabilar
Copy link
Member

@kabilar kabilar commented Jan 30, 2025

Fixes #1463

Edit:
We encountered this issue again today with dandi==0.66.4 so I suggest that we pin to the latest dandischema version.

Error message:
$ dandi upload -i linc <path_to_ome_zarr>
Error: Server requires schema version 0.6.8; client only supports 0.6.9. You may need to upgrade dandi and/or dandischema

cc @aaronkanzer @dstansby

@kabilar kabilar requested a review from jwodder January 30, 2025 15:00
Copy link

codecov bot commented Jan 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.44%. Comparing base (05f5691) to head (360455f).
Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1570      +/-   ##
==========================================
+ Coverage   88.34%   88.44%   +0.10%     
==========================================
  Files          78       78              
  Lines       10735    10735              
==========================================
+ Hits         9484     9495      +11     
+ Misses       1251     1240      -11     
Flag Coverage Δ
unittests 88.44% <ø> (+0.10%) ⬆️

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.

@jwodder
Copy link
Member

jwodder commented Jan 30, 2025

@kabilar This seems to be solving the wrong problem. If the client's schema version is newer than the server's — as the given error message states — then pinning to the latest schema version will just make it so users can't downgrade their dandischema to an older version to solve the problem.

@kabilar
Copy link
Member Author

kabilar commented Jan 30, 2025

@kabilar This seems to be solving the wrong problem. If the client's schema version is newer than the server's — as the given error message states — then pinning to the latest schema version will just make it so users can't downgrade their dandischema to an older version to solve the problem.

Thanks @jwodder. Good catch. I misread the error message.

@kabilar
Copy link
Member Author

kabilar commented Jan 30, 2025

We will still upgrade the linc-archive dandischema version.

@jwodder Would it be worth it to go ahead with this pull request to pin the dandischema version as the dandi-archive does to address #1463?

From dandi-archive setup.py line 45:

'dandischema==0.11.0',  # schema version 0.6.9

@jwodder
Copy link
Member

jwodder commented Jan 30, 2025

@kabilar I don't like the idea of pinning dandischema. What if a new dandischema version is released that doesn't change the schema version but still fixes some bug? If you really need dandischema to only be for the latest schema version, make the requirement be a range with the lower end being the point when that schema version was added.

@kabilar
Copy link
Member Author

kabilar commented Jan 30, 2025

@kabilar I don't like the idea of pinning dandischema. What if a new dandischema version is released that doesn't change the schema version but still fixes some bug?

Thats fair.

If you really need dandischema to only be for the latest schema version, make the requirement be a range with the lower end being the point when that schema version was added.

So in this case would dandischema>=0.11.0 work?

@jwodder
Copy link
Member

jwodder commented Jan 30, 2025

@kabilar I think that should be dandischema>=0.11.0,<0.12.0.

@kabilar
Copy link
Member Author

kabilar commented Jan 30, 2025

@kabilar I think that should be dandischema>=0.11.0,<0.12.0.

Sounds good. I have updated it.

@jwodder jwodder added the dependencies Update one or more dependencies version label Jan 30, 2025
@kabilar kabilar changed the title Pin dandischema to require the latest version Pin dandischema to require the latest schema version Jan 30, 2025
@yarikoptic
Copy link
Member

tests are failing, fomr a sample run

FAILED dandi/tests/test_dandiarchive.py::test_parse_api_url[https://gui.dandiarchive.org/#/dandiset/000001/0.201104.2302-parsed_url2] - requests.exceptions.ConnectionError: ('Connection aborted.', ConnectionResetError(104, 'Connection reset by peer'))
FAILED dandi/tests/test_dandiarchive.py::test_follow_redirect - requests.exceptions.ConnectionError: ('Connection aborted.', ConnectionResetError(104, 'Connection reset by peer'))

looks like unrelated but that should be figured out (e.g. separate PR showing the same fails), and ideally fixed... FWIW yesterday's run seems had failed only in a single run https://github.com/dandi/dandi-cli/actions/runs/13025543642 , so more analysis should be done... assuming it was connection issue, I would restart failing tests now, so see if that would be gone

@kabilar
Copy link
Member Author

kabilar commented Jan 30, 2025

Not sure what changed, but it looks like all tests are passing now.

@yarikoptic yarikoptic added the release Create a release when this pr is merged label Jan 30, 2025
@yarikoptic yarikoptic merged commit fd0cb50 into dandi:master Jan 30, 2025
24 checks passed
@yarikoptic
Copy link
Member

@kabilar probably they just flaked out due to connection issues or other external factors (redirects etc... as we were observing similar ones in upptime checks)

Copy link

🚀 PR was released in 0.66.5 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Update one or more dependencies version release Create a release when this pr is merged released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Schema mismatch between server and client
3 participants