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

Fix #350, specify JSON schema draft version #431

Merged
merged 2 commits into from
Aug 24, 2019

Conversation

davidlatwe
Copy link
Collaborator

Changes

  • Specifying our schema draft version to version 4, because the module jsonschema we vendorized was only supported up to version 4.

  • Change keyword const to enum, because const was introduced in JSON schema Draft 6.

You may see #350 for detail :)

davidlatwe and others added 2 commits January 21, 2019 20:18
The validation keyword `const` was from Draft06, but the current `jsonschema` version in `avalon.vendor` is `2.4.0`, which only support to Draft04 validation, so `const` will not validate as expect.

Changing to `enum` will fix this.
@mottosso
Copy link
Contributor

Nice one! For completeness, our vendored version of jsonschema won't actually take this into account, but once/if we update then these constraints should kick in? Or do they work already?

@davidlatwe
Copy link
Collaborator Author

Yeah, the keyword const was not working, for example, you could change the container id from pyblish.avalon.container into arbitrary string and it still pass the validation.

For example:

from avalon.vendor import jsonschema

data = {"foo": "bar"}
schema = {"properties": {"foo": {"const": "not-bar"}}}
jsonschema.validate(data, schema)
# No error raised

@mottosso
Copy link
Contributor

Aaah so the current schema was using a too new keyword, and this brings it back to the version we're actually using to validate. Gotcha, that makes sense. Happy with this!

@mottosso mottosso merged commit 60da0a0 into getavalon:master Aug 24, 2019
@davidlatwe davidlatwe deleted the jsonschema branch January 14, 2020 07:46
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