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

Update to jsonschema > 4.18, Sphinx < 7 #572

Merged
merged 2 commits into from
Aug 29, 2023
Merged

Conversation

HGSilveri
Copy link
Collaborator

I suspect this is what's causing problems in the docs build, so I'm switching to the reference system introduced in jsonschema 4.18.

@HGSilveri HGSilveri requested a review from a-corni August 28, 2023 15:52
@HGSilveri HGSilveri changed the title Update to jsonschema > 4.18 Update to jsonschema > 4.18, Sphinx < 7 Aug 28, 2023
@HGSilveri
Copy link
Collaborator Author

Turns out it was the Sphinx version. It's too complex for me to figure out exactly why, but restricting to <7 solves the issue.

@HGSilveri HGSilveri marked this pull request as ready for review August 28, 2023 16:52
Copy link
Collaborator

@a-corni a-corni left a comment

Choose a reason for hiding this comment

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

I indeed don't think that jsonschema is responsible for our issues, since I made a commit that passed on August 8th whereas their latest change was on August 7th. Yet, it's good to make these changes, but I have a question regarding them: I see the resource taken for the registry is SCHEMAS["device"] whereas we used to take the SCHEMAS["sequence"] as referring document in the `RefResolver. Why this difference ? Why is it working ?

base_uri=f"{SCHEMAS_PATH.resolve().as_uri()}/",
referrer=SCHEMAS["sequence"],
REGISTRY: Registry = Registry().with_resources(
[("device-schema.json", Resource.from_contents(SCHEMAS["device"]))]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is it device and not sequence ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tbh, I think I never quite understood what the referrer argument did (in their docs, they have an example where they set it to True 🙃), I just went with something that worked. The schema being referred and the name (ie device-schema.json) was taken from the path in the base_uri.
Now, what we are doing is specifically building a Registry with the schemas that can be referred and the name they are referred by.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay I just had a quick look at the doc but did not go further, I understood the role of the Registry and the Resource so let's go with this ;)

base_uri=f"{SCHEMAS_PATH.resolve().as_uri()}/",
referrer=SCHEMAS["sequence"],
REGISTRY: Registry = Registry().with_resources(
[("device-schema.json", Resource.from_contents(SCHEMAS["device"]))]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay I just had a quick look at the doc but did not go further, I understood the role of the Registry and the Resource so let's go with this ;)

@HGSilveri HGSilveri merged commit f742f5c into develop Aug 29, 2023
6 checks passed
@HGSilveri HGSilveri deleted the hs/update-jsonschema branch August 29, 2023 09:16
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.

2 participants