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

Upgrade jsonschema #1477

Merged
merged 6 commits into from Nov 6, 2023
Merged

Upgrade jsonschema #1477

merged 6 commits into from Nov 6, 2023

Conversation

SpacemanPaul
Copy link
Contributor

@SpacemanPaul SpacemanPaul commented Jul 24, 2023

Reason for this pull request

A jsonschema deprecation warning was raised in #1474.

This turned out to be due to the new "referencing" library released in jsonschema 4.18.0 replacing the old RefResolver API, which was deprecated from 4.18.0.

I pinned jsonschema<4.18 in #1476 as an immediate fix. This PR is a longer term fix - pinning to jsonschema>=4.18 switching to the new API.

4.18.0 was released barely 2 weeks ago at the time of writing and there have been 4 minor releases since, hence merging of this was postponed until the rest of the ecosystem calmed down.

Proposed changes

  • Migrate the validate_document decorator to use the new referencing API.

  • Pin jsonschema>=4.18

  • Tests added / passed

  • Fully documented, including docs/about/whats_new.rst for all changes


📚 Documentation preview 📚: https://datacube-core--1477.org.readthedocs.build/en/1477/

@codecov
Copy link

codecov bot commented Jul 24, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (0cfe300) 91.75% compared to head (79ff588) 91.75%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1477      +/-   ##
===========================================
- Coverage    91.75%   91.75%   -0.01%     
===========================================
  Files          132      132              
  Lines        14549    14552       +3     
===========================================
+ Hits         13350    13352       +2     
- Misses        1199     1200       +1     
Files Coverage Δ
datacube/utils/documents.py 98.04% <83.33%> (-0.31%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SpacemanPaul SpacemanPaul marked this pull request as ready for review July 24, 2023 03:30
@SpacemanPaul SpacemanPaul requested a review from omad July 24, 2023 03:30
SpacemanPaul added a commit that referenced this pull request Jul 24, 2023
@emmaai
Copy link
Contributor

emmaai commented Sep 8, 2023

This will cause trouble in all stac related functionalities. I did raise an issue in pystac repo, but ideally they'd be very happy if I could raise a pr fixing it. ref stac-utils/pystac#1214

@SpacemanPaul
Copy link
Contributor Author

I'm beginning to think we should keep both implementations for the time being - dynamically try the new API first and fallback to the old API if the new one isn't available - that way we can support everything while the rest of the ecosystem dukes it out.

@emmaai
Copy link
Contributor

emmaai commented Sep 21, 2023

the jsonschema issue with pystac was resolved. maybe it's time for us to merge the change.

@emmaai
Copy link
Contributor

emmaai commented Nov 1, 2023

@SpacemanPaul any plan to merge this soon?

@SpacemanPaul
Copy link
Contributor Author

SpacemanPaul commented Nov 1, 2023

@SpacemanPaul any plan to merge this soon?

Yep, it's on my todo list - will definitely be in the next release. (although not sure when that will be.)

Copy link
Contributor

@emmaai emmaai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@emmaai emmaai merged commit 43903ea into develop Nov 6, 2023
31 checks passed
@emmaai emmaai deleted the upgrade_jsonschema branch November 6, 2023 02:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants