Work towards compatibility for 4.17 and 4.18 #4

braingram · 2023-03-20T20:10:53Z

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:

FAILED asdf/_tests/test_schema.py::test_load_schema - referencing.exceptions.NoSuchAnchor: 'definitions/stringArray' does not exist within {'id': 'http://json-schema.org/draft-04/schema#', '$schema': 'http://json-schema.org/draft-04/schema#', 'description': 'Core schema meta-schema', 'definiti...
FAILED asdf/_tests/test_schema.py::test_load_schema_with_full_tag - referencing.exceptions.NoSuchAnchor: 'definitions/stringArray' does not exist within {'id': 'http://json-schema.org/draft-04/schema#', '$schema': 'http://json-schema.org/draft-04/schema#', 'description': 'Core schema meta-schema', 'definiti...
FAILED asdf/_tests/test_schema.py::test_load_schema_with_tag_address - referencing.exceptions.NoSuchAnchor: 'definitions/stringArray' does not exist within {'id': 'http://json-schema.org/draft-04/schema#', '$schema': 'http://json-schema.org/draft-04/schema#', 'description': 'Core schema meta-schema', 'definiti...
FAILED asdf/_tests/test_schema.py::test_load_schema_with_file_url - referencing.exceptions.NoSuchAnchor: 'definitions/stringArray' does not exist within {'id': 'http://json-schema.org/draft-04/schema#', '$schema': 'http://json-schema.org/draft-04/schema#', 'description': 'Core schema meta-schema', 'definiti...

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.

braingram · 2023-03-20T20:12:28Z

asdf/schema.py

+                            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)


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

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-commenter · 2023-03-20T20:21:05Z

Codecov Report

❗ No coverage uploaded for pull request base (eslavich-no-patch-iter-errors@b4e9f35). Click here to learn what that means.
The diff coverage is n/a.

📣 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.

eslavich

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.

eslavich · 2023-03-21T04:07:35Z

asdf/resource.py

@@ -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}")


Ahhhh so that's where that got to! I was super confused when I didn't see it in either referencing or jsonschema.

eslavich · 2023-03-21T04:08:07Z

asdf/resource.py

-_JSONSCHEMA_URI_TO_FILENAME = {
-    "http://json-schema.org/draft-04/schema": "draft4.json",
-}
+if importlib_metadata.version('jsonschema') >= '4.18':


I guess we should make this 4.18.0dev...

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.

eslavich · 2023-03-21T04:10:01Z

asdf/schema.py

+if importlib_metadata.version('jsonschema') >= "4.18":
+    USE_REFERENCING = True
+    import referencing
+    from referencing.exceptions import Unretrievable as RefError


I believe there are actually two exceptions we need to catch here, Unretrievable and Unresolvable.

eslavich · 2023-03-21T04:17:02Z

asdf/schema.py

+                            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)


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.

eslavich · 2023-03-21T04:18:45Z

asdf/schema.py

-                    yield from instance_validator.descend(instance["default"], instance)
-                finally:
-                    instance_validator.resolver.pop_scope()
+                get_validator(instance).validate(instance['default'])


Well that certainly seems more better!

braingram added 4 commits March 20, 2023 15:27

WIP: jsonschema 4.17/4.18 check_schema not working

ca43ea4

WIP: fixed jsonschema 4.18 schema path

7080d41

WIP: 4 failing tests with 4.18

291c008

some code cleanup

4f13930

braingram commented Mar 20, 2023

View reviewed changes

eslavich approved these changes Mar 21, 2023

View reviewed changes

eslavich marked this pull request as ready for review March 21, 2023 04:22

eslavich merged commit 8f6a922 into eslavich:eslavich-no-patch-iter-errors Mar 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Work towards compatibility for 4.17 and 4.18 #4

Work towards compatibility for 4.17 and 4.18 #4

braingram commented Mar 20, 2023

braingram Mar 20, 2023

eslavich Mar 21, 2023

codecov-commenter commented Mar 20, 2023 •

edited

eslavich left a comment

eslavich Mar 21, 2023

eslavich Mar 21, 2023

eslavich Mar 21, 2023

eslavich Mar 21, 2023

eslavich Mar 21, 2023

eslavich Mar 21, 2023

Work towards compatibility for 4.17 and 4.18 #4

Work towards compatibility for 4.17 and 4.18 #4

Conversation

braingram commented Mar 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Mar 20, 2023 • edited

Codecov Report

eslavich left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Mar 20, 2023 •

edited