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

Avalon's schema validation module does not support Draft-6 validation keywords #350

Closed
davidlatwe opened this issue Nov 5, 2018 · 6 comments

Comments

@davidlatwe
Copy link
Collaborator

Problem

Some of Avalon's schema were using the keyword const, which was introduced in JSON schema Draft 6, which require jsonschema==3.0.0 to work, and the current vendoring version of jsonschema in Avalon is version 2.4.0.

Those not supported keyword will not functioning, which means no jsonschema.ValidationError raised.

See the following example:

from avalon.vendor import jsonschema

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

Above example code will raise jsonschema.ValidationError if you use the latest version (3.0.0) downloaded from jsonschema github repo.

Note: version 3.0.0 only on github, the latest on pip is 2.6.0, which still not support Draft 6.
Difference between Draft4 and Draft6

So, if someone changed the default value which provided by Avalon, and thought it's okay since no error raised, will be an issue in the feature, when Avalon update the jsonschema vendor module, someone's pipeline will broke.

For example, in my case, I changed the container id from pyblish.avalon.container to pyblish.avalon.sub-container, for assets like setdress type, which has nested container.

Implementation

Eventually, Avalon will update jsonschema vendor module at some point, and when the time comes, we could let TDs to use Draft4Validator for the time being and pop warning.

Example code:

import os
import warnings
from avalon.vendor import jsonschema  # assume it's version 3.0


def validate(data, schema):
    SCHEMA_COMPAT = bool(os.environ.get("AVALON_SCHEMA_COMPAT"))

    try:
        jsonschema.validate(data, schema)  # validating with latest Draft
    except jsonschema.ValidationError as e:
        if SCHEMA_COMPAT:
            warnings.warn("Validating with Draft4...")
            validator = jsonschema.Draft4Validator(schema)
            validator.validate(data)
        else:
            raise e
@mottosso
Copy link
Contributor

mottosso commented Nov 5, 2018

Happy to see a PR for this; the tests should kick in to see if anything immediately breaks.

@davidlatwe
Copy link
Collaborator Author

davidlatwe commented Nov 5, 2018

Dumping additional note:

  • jsonschema.validate(instance, schema, cls)

    If the cls argument is not provided, two things will happen in accordance with the specification. First, if the schema has a $schema property containing a known meta-schema then the proper validator will be used. The specification recommends that all schemas contain $schema properties for this reason. If no $schema property is found, the default validator class is the latest released draft.

Currently all the schemas are having this meta-schema http://json-schema.org/schema#, if we are going to bumping schema version at some point in the future, perhaps considering change the old version's $schema to http://json-schema.org/draft-04/schema#, for clarity.

In fact, maybe we could solving this issue by changing all meta-schema to http://json-schema.org/draft-04/schema#, which will make the jsonschema.validator to chose Draft4Validator to validate automagicly ! Then we can safely update the jsonschema version without changing other code. And if we want to really use the keywords like const from Draft 6, we make new version of schema.

I think this is the best option :)

@davidlatwe
Copy link
Collaborator Author

davidlatwe commented Nov 5, 2018

In fact, maybe we could solving this issue by changing all meta-schema to http://json-schema.org/draft-04/schema#, which will make the jsonschema.validator to chose Draft4Validator to validate automagicly ! Then we can safely update the jsonschema version without changing other code. And if we want to really use the keywords like const from Draft 6, we make new version of schema.

I think this is the best option :)

Example:

After update the jsonschema...

  • With current $schema
import jsonschema  # version 3.0

data = {"foo": "bar"}
schema = {"$schema": "http://json-schema.org/schema#",
          "properties": {"foo": {"const": "foobar"}}}
jsonschema.validate(data, schema)  # Auto validating with latest Draft
# Error raised
  • Specifically change $schema to Draft 4
import jsonschema  # version 3.0

data = {"foo": "bar"}
schema = {"$schema": "http://json-schema.org/draft-04/schema#",
          "properties": {"foo": {"const": "foobar"}}}
jsonschema.validate(data, schema)  # Auto validating with Draft 4
# No error :)

@davidlatwe
Copy link
Collaborator Author

davidlatwe commented Nov 9, 2018

We might have to remove jsonschema from vendoring for update to support Draft06,
jsonschema added one more required module pyrsistent in v3.0.0a3 (commit 644259b).

install_requires =
    attrs>=17.4.0
    pyrsistent>=0.14.0
    six>=1.11.0
    functools32;python_version<'3'

@mottosso
Copy link
Contributor

We might have to remove jsonschema from vendoring for update to support Draft06,

If it means asking the end-user to install an additional dependency, it's not worth the gain of validating against const. Especially if it hasn't been a problem already, has it?

@davidlatwe
Copy link
Collaborator Author

davidlatwe commented Jan 21, 2019

Especially if it hasn't been a problem already, has it?

Not affect to production in most cases, yes.
But if someone changed, for example, the container id like pyblish.avalon.container to something else because of a typo or any other human-err, the validation won't spot it and something might break in the future.

it's not worth the gain of validating against const.

Yeah, updating jsonschema to version 3 for Draft06 supporting const is not a must, we could change the validation keyword from const to enum as alternative for now, for example:

from avalon.vendor import jsonschema

# Validation: The value of "foo" must be "bar"

data = {"foo": "not-bar"}
schema = {"$schema": "http://json-schema.org/schema#",
          "properties": {"foo": {"enum": ["bar"]}}}  # Use `enum` instead of `const`
jsonschema.validate(data, schema)
# Error raised

And for clarity, I suggest we also change the value of $schema to "http://json-schema.org/draft-04/schema#", for specifying that we are currently using Draft04.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants