getavalon / core

The safe post-production pipeline - https://getavalon.github.io/2.0
MIT License
218 stars 48 forks source link

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

Closed davidlatwe closed 5 years ago

davidlatwe commented 5 years ago

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 commented 5 years ago

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

davidlatwe commented 5 years ago

Dumping additional note:

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 commented 5 years ago

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...

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
```python
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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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.