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

Using "str: object" to allow and ignore unrecognized fields defeats validation of Optional fields #57

Closed
jab opened this issue Mar 22, 2015 · 7 comments

Comments

@jab
Copy link
Contributor

jab commented Mar 22, 2015

Use case: A schema may contain some optional fields that should be validated if present. Unrecognized fields should be allowed and ignored.

The docs say that unrecognized fields may be allowed and ignored by including str: object in the schema. However, this defeats the validation of any Optional fields:

Python 3.4.3 (default, Mar  1 2015, 15:44:16)
[GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.56)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from schema import Schema, Optional
>>> s = Schema({'required': str, Optional('optional'): int})
>>> s.validate({'required': 'valid', 'optional': 'invalid'})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.4/site-packages/schema.py", line 125, in validate
    nvalue = Schema(svalue, error=e).validate(value)
  File "/usr/local/lib/python3.4/site-packages/schema.py", line 161, in validate
    raise SchemaError('%r should be instance of %r' % (data, s), e)
schema.SchemaError: 'invalid' should be instance of <class 'int'>
>>> s = Schema({'required': str, Optional('optional'): int, str: object})
>>> s.validate({'required': 'valid', 'optional': 'invalid'})
{'required': 'valid', 'optional': 'invalid'}
>>> from schema import __version__
>>> __version__
'0.3.1'

Is this behavior intentional or is this a bug?

Thanks for any attention to this and for releasing schema.

P.S. I cranked out some one-off code to do schema validation in a side project years ago that I never separated out and released as its own module. It's still up on GitHub at https://github.com/jab/SiCDS/blob/master/sicds/schema.py in case there's anything there you'd like to incorporate into this project. Looks like I released that code GPLv3 but if you'd require another license I could certainly relicense it.

@sjakobi
Copy link
Contributor

sjakobi commented Sep 25, 2015

I'm getting an error today (e94b714) for the second example:

schema.SchemaError: 'invalid' should be instance of 'int'

Is this the behaviour you would have expected?

I believe the relevant change was ed80757.

@skorokithakis
Copy link
Collaborator

In my reading, I believe that the original bug report has been fixed, as you mention, @sjakobi. I will close this, feel free to reopen it, @jab, if you think it has been closed in error.

@jab
Copy link
Contributor Author

jab commented Sep 29, 2015

Just ran pip install --upgrade 'git+https://github.com/keleshev/schema.git', tried my original example again, and confirmed it now gives SchemaError as it should have:

Python 3.5.0 (default, Sep 14 2015, 00:19:46)
[GCC 4.2.1 Compatible Apple LLVM 6.1.0 (clang-602.0.53)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from schema import Schema, Optional
>>> s = Schema({'required': str, Optional('optional'): int, str: object})
>>> s.validate({'required': 'valid', 'optional': 'invalid'})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/var/pyenv/versions/wam-py35/lib/python3.5/site-packages/schema.py", line 131, in validate
    nvalue = Schema(svalue, error=e).validate(value)
  File "/usr/local/var/pyenv/versions/wam-py35/lib/python3.5/site-packages/schema.py", line 169, in validate
    (data, s.__name__), e)
schema.SchemaError: 'invalid' should be instance of 'int'

Thanks for fixing!

Any sense of how long before we can expect a release with the fix?

And any sense for how stable latest master is / would you advise against users running against it in production?

P.S. In the process of testing this I hit #90 -- hope that PR helps.

@jab
Copy link
Contributor Author

jab commented Sep 30, 2015

Oh, just noticed #59, good to read a new release sounds imminent. Will follow that issue to find out when it happens.

In the meantime, would love to know if anyone had a chance to check out the related code I linked to above. If there's anything schema can use from that, I'd be happy to try to facilitate that. @keleshev @skorokithakis @sjakobi @kosz85 et al.

@kosz85
Copy link

kosz85 commented Sep 30, 2015

@jab I like your nice and simple interface. Thought this schema has simple interface too. I love to work with attributes like in your solution (as it's more natural), but I myself would use it as proxy or wrapper for schema and not as feature of main schema object. Attrs have many constrictions if we talk about their names, so not for everyone it's fully usable. You would need to add some auto-mapping (and manual too) for keys with spaces and special chars. But in future I would love to see such proxy also in this project, but probably as smth separate.

@skorokithakis
Copy link
Collaborator

@jab That is indeed a very nice, simple interface. I think it's a bit limited in that you can't easily specify subschemas (you have to compose them with other classes, which is a bit verbose), but it's a nice alternative. I don't think it does the same thing as this package (this package is more for imposing schemas on data, yours is for imposing schemas on classes), so I think both have their place in a project. I don't think they're interchangeable, or even very related in the ways they'll be used.

@jab
Copy link
Contributor Author

jab commented Oct 1, 2015

Thanks for taking a look. Just to clear up some misunderstandings,

You would need to add some auto-mapping (and manual too) for keys with spaces and special chars.

Nope, those work fine:

>>> from schema import *
>>> class Foo(Schema):
...     required = {'key with spaces and special chars!': int}
...
>>> foo = Foo({'key with spaces and special chars!': 5})
>>> foo = Foo({'key with spaces and special chars!': 'invalid'})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "schema.py", line 206, in __init__
    value = self._validate(field, validator, value)
  File "schema.py", line 188, in _validate
    raise InvalidField(field, value)
schema.InvalidField: ('key with spaces and special chars!', 'invalid')

I don't think it does the same thing as this package (this package is more for imposing schemas on data, yours is for imposing schemas on classes

Nope, mine is for data, you misunderstood.

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

No branches or pull requests

4 participants