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

Upgrade CI scripts dependencies #1786

Merged
merged 6 commits into from
Apr 16, 2024
Merged

Conversation

zecakeh
Copy link
Contributor

@zecakeh zecakeh commented Apr 16, 2024

Will allow us to benefit from future fixes in JSON Schema validation.

Preview: https://pr1786--matrix-spec-previews.netlify.app

Port to the use of the referencing library
instead of the deprecated RefResolver

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
JSONPath is now a RFC instead of a draft so the major release was bumped

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
@zecakeh zecakeh requested a review from a team as a code owner April 16, 2024 09:49
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

Looks sensible other than a couple of nits

@@ -42,6 +42,12 @@ def import_error(module, package, debian, error):
import_error("jsonschema", "jsonschema", "jsonschema", e)
raise

try:
import referencing
Copy link
Member

Choose a reason for hiding this comment

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

Since we depend on it, we should add this to requirements.txt, even though it will be pulled in as a transitive dependency in practice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added


def load_resource_from_uri(path):
"""Load a JSON or YAML JSON Schema from a file:// URI.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"""Load a JSON or YAML JSON Schema from a file:// URI.
"""Load a JSON or YAML JSON Schema, as a `referencing.Resource` object, from a file:// URI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

Thanks!

@richvdh richvdh merged commit a81b720 into matrix-org:main Apr 16, 2024
12 checks passed
@zecakeh zecakeh deleted the upgrade-scripts-deps branch April 16, 2024 17:51
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.

None yet

2 participants