-
Notifications
You must be signed in to change notification settings - Fork 0
Work towards compatibility for 4.17 and 4.18 #4
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
Work towards compatibility for 4.17 and 4.18 #4
Conversation
tag_resource = self._resolver.get_or_retrieve(schema_uri) | ||
resolver = tag_resource.value @ self._resolver | ||
v = self._json_schema_validator_factory.create(tag_resource.value.contents, resolver) | ||
v._resolver = resolver.resolver_with_root(tag_resource.value) |
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.
This was added when trying to debug why many tests were failing with errors like:
referencing.exceptions.Unresolvable: software-1.0.0
which was tracked down to loss of the base_uri
I think here:
https://github.com/python-jsonschema/jsonschema/blob/529b57aefb4810866c3de0750349fb227fac816e/jsonschema/validators.py#L248-L252
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.
This I believe is a symptom of this issue:
python-jsonschema/jsonschema#1061
Once we're able to specify the dialect we want, that specification object will be able to find the software schema's id field and use it as the base URI.
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## eslavich-no-patch-iter-errors #4 +/- ##
================================================================
Coverage ? 96.05%
================================================================
Files ? 97
Lines ? 12280
Branches ? 0
================================================================
Hits ? 11796
Misses ? 484
Partials ? 0 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
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.
I'd like to do some work on the branch tonight without creating conflicts for you, so I'll go ahead and merge this. The only comment of any consequence is the thing about the exception classes but that's an easy follow up.
@@ -181,7 +189,10 @@ class JsonschemaResourceMapping(Mapping): | |||
|
|||
def __getitem__(self, uri): | |||
filename = _JSONSCHEMA_URI_TO_FILENAME[uri] | |||
return pkgutil.get_data("jsonschema", f"schemas/{filename}") | |||
if USE_JSONSCHEMA_SPECIFICATIONS: | |||
return pkgutil.get_data("jsonschema_specifications", f"schemas/draft4/{filename}") |
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.
Ahhhh so that's where that got to! I was super confused when I didn't see it in either referencing or jsonschema.
_JSONSCHEMA_URI_TO_FILENAME = { | ||
"http://json-schema.org/draft-04/schema": "draft4.json", | ||
} | ||
if importlib_metadata.version('jsonschema') >= '4.18': |
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.
I guess we should make this 4.18.0dev...
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.
Oops, another thing I didn't notice until after I merged is we're just comparing strings here. Need to use packaging.version.parse so that they get compared as version numbers.
if importlib_metadata.version('jsonschema') >= "4.18": | ||
USE_REFERENCING = True | ||
import referencing | ||
from referencing.exceptions import Unretrievable as RefError |
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.
I believe there are actually two exceptions we need to catch here, Unretrievable
and Unresolvable
.
tag_resource = self._resolver.get_or_retrieve(schema_uri) | ||
resolver = tag_resource.value @ self._resolver | ||
v = self._json_schema_validator_factory.create(tag_resource.value.contents, resolver) | ||
v._resolver = resolver.resolver_with_root(tag_resource.value) |
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.
This I believe is a symptom of this issue:
python-jsonschema/jsonschema#1061
Once we're able to specify the dialect we want, that specification object will be able to find the software schema's id field and use it as the base URI.
yield from instance_validator.descend(instance["default"], instance) | ||
finally: | ||
instance_validator.resolver.pop_scope() | ||
get_validator(instance).validate(instance['default']) |
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.
Well that certainly seems more better!
I spent today working on testing out your no-patch-iter-errors branch and made some attempts at getting it working with 4.17 and 4.18.
A few tests are still failing on 4.18:
I'll also call out one obvious use of private api that I added to get around the validation sort of losing track of the base uri. I'm not sure if this is because of something we're doing wrong or a bug in jsonschema.