Skip to content

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

zecakeh added 2 commits April 16, 2024 11:46
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
zecakeh added 2 commits April 16, 2024 11:52
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

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

return yaml.safe_load(f)

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

zecakeh added 2 commits April 16, 2024 17:13
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
@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.

2 participants