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

Use jsonschema ≥4.18.0 and new referencing library #1691

Merged
merged 2 commits into from
Dec 9, 2024

Conversation

victorlin
Copy link
Member

@victorlin victorlin commented Dec 5, 2024

Description of proposed changes

In v4.18.0, jsonschema.RefResolver was deprecated in favor of the new referencing library. The intro and API docs were helpful in determining the necessary changes.

I've tested that our new usage is not backwards compatible with v4.17.3 and thus updated the minimum requirement to v4.18.0. I chose v0.29.1 as the minimum supported version of referencing because that was the version released alongside jsonschema v4.18.0.

The default behavior no longer tries to access the network, so I've reworded the retrieval function comment and error message.

Local reference mismatches are now a "PointerToNowhere" error instead of an "Unresolvable JSON pointer" error. It shows the entire schema JSON in the output which can seem unnecessarily verbose, but I think it's fine since this is only intended to show on internal errors with the schema.

Related issue(s)

Closes #1358

Checklist

  • Automated checks pass
  • Check if you need to add a changelog message
  • Check if you need to add tests
  • Check if you need to update docs

@victorlin victorlin self-assigned this Dec 5, 2024
Copy link

codecov bot commented Dec 5, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 72.30%. Comparing base (e90383b) to head (901fcc8).
Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
augur/validate.py 80.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1691   +/-   ##
=======================================
  Coverage   72.30%   72.30%           
=======================================
  Files          79       79           
  Lines        8276     8276           
  Branches     1691     1691           
=======================================
  Hits         5984     5984           
  Misses       2007     2007           
  Partials      285      285           

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

@huddlej
Copy link
Contributor

huddlej commented Dec 5, 2024

Thanks, @victorlin! I love an improvement that's +5 lines and -14 😄

Just adding a note here for our own reference that we'll need to add referencing to the Bioconda package dependencies and update the jsonschema version pin there.

@victorlin victorlin force-pushed the victorlin/jsonschema-v4 branch from 0292963 to 2e8fc63 Compare December 5, 2024 19:07
@victorlin victorlin force-pushed the victorlin/jsonschema-v4 branch from 2e8fc63 to 9e33b0e Compare December 5, 2024 19:32
CHANGES.md Outdated Show resolved Hide resolved
@victorlin victorlin force-pushed the victorlin/jsonschema-v4 branch from 8b3e456 to 67ada11 Compare December 5, 2024 19:58
@victorlin victorlin added the breaking Makes a backwards incompatible change and should wait for major release label Dec 5, 2024
augur/validate.py Show resolved Hide resolved
@victorlin victorlin force-pushed the victorlin/jsonschema-v4 branch 2 times, most recently from 86a23c1 to 26a76f9 Compare December 6, 2024 20:36
In v4.18.0, jsonschema.RefResolver was deprecated in favor of the new
referencing library.¹ The intro² and API³ docs were helpful in
determining the necessary changes.

I've tested that our new usage is not backwards compatible with v4.17.3
and thus updated the minimum requirement to v4.18.0. I chose v0.29.1 as
the minimum supported version of referencing because that was the
version released alongside jsonschema v4.18.0.

The default behavior no longer tries to access the network, so I've
reworded the retrieval function comment and error message.

Local reference mismatches are now a "PointerToNowhere" error instead of
an "Unresolvable JSON pointer" error. It shows the entire schema JSON in
the output which can seem unnecessarily verbose, but I think it's fine
since this is only intended to show on internal errors with the schema.

¹ https://github.com/python-jsonschema/jsonschema/blob/93e0caa5752947ec77333da81a634afe41a022ed/CHANGELOG.rst#v4180
² https://python-jsonschema.readthedocs.io/en/stable/referencing/#introduction-to-the-referencing-api
³ https://referencing.readthedocs.io/en/stable/api/#referencing.Registry.with_contents
@victorlin victorlin force-pushed the victorlin/jsonschema-v4 branch from 26a76f9 to 901fcc8 Compare December 6, 2024 21:26
@victorlin victorlin dismissed jameshadfield’s stale review December 9, 2024 18:03

addressed requested changes

@victorlin victorlin merged commit 037525d into master Dec 9, 2024
35 checks passed
@victorlin victorlin deleted the victorlin/jsonschema-v4 branch December 9, 2024 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Makes a backwards incompatible change and should wait for major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

export crashes with KeyError: 'tree' when jsonschema v4 is installed
4 participants