-
Notifications
You must be signed in to change notification settings - Fork 58
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
Attempt to use only public jsonschema interfaces #1490
Attempt to use only public jsonschema interfaces #1490
Conversation
43031bd
to
33f54ab
Compare
@eslavich this looks like it will work really well, and the changes look nice at first glance. However, I have not had a chance to look over the changes in detail. Since you are still testing this could you remove the jsonschema upper pin? It would be nice to go ahead and fix that issue. |
This PR doesn't actually fix our issues with the jsonschema alpha release, it just positions us better to do so. That'll be coming up in a subsequent PR. I wouldn't recommend doing a detailed review of this one yet, I need to rework part of it for performance reasons... |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
5be2e5e
to
607a744
Compare
I think the asdf-astropy (and possibly the dkist errors) will be fixed by:
To summarize things so far, it looks like this PR should allow compatibility with jsonschema 4.18. However, the more strict schema parsing in this newer version means that asdf will be unable to validate using schema for the current released versions of:
There are also the issues you opened in jsonschema that should hopefully be addressed before we merge these changes: We are also now depending on |
Braingram no patch iter errors
I've been futzing with this tonight, trying to understand the consequences of the immutable Registry object. It seems like when jsonschema.Validator makes a lookup to descend into a referenced schema: https://github.com/python-jsonschema/jsonschema/blob/main/jsonschema/validators.py#L404 it loses the work that has been done to load our schema. That includes YAML parsing in our retrieve callback as well as whatever the Registry.crawl() method does. I guess that implies we'd better put a cache decorator on our retrieve method? |
I worked up a minimal case for the registry and opened an issue with jsonschema: |
add cache to get_schema, combine instance checks with validate
This has been superseded by #1591 |
Just checking if this cuts the mustard with our downstream packages, stay tuned...