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

Schema aren't validated when constructing a Validator #39

Closed
ch3pjw opened this Issue May 22, 2014 · 4 comments

Comments

Projects
None yet
2 participants
@ch3pjw
Copy link

ch3pjw commented May 22, 2014

This issue is more a matter of philosphy than a bug, but I thought I'd write it down anyway.

Impact:

If a schema contains an error then the validating program using the schema will only crash if it receives data pertaining to the erroneous part of the schema. This can make debugging hard.

Reproduction:
import cerberus
schema = {
    'ok': {'type': 'string'},
    'poop': {'not_valid_key_for_schema': 'wrong'}}
validator = cerberus.Validator(schema)  # Works fine even though validation of 'poop' will fail
validator.validate({'ok': 'everything is awesome'}
validator.validate({'poop': 'ah, it broke'})
Suggested behaviour:

I wonder whether when creating a validator, or updating it's schema, cerberus should validate the schema itself against an internal schema for what schema definitions can look like. This would avoid only hitting mistakes in schema definition when runtime data hits that part of the schema, which could be quite rare.

Penny for you thoughts?

@nicolaiarocci

This comment has been minimized.

Copy link
Member

nicolaiarocci commented May 22, 2014

Agree that it is going to be very rare. Also, at quick glance maintaining a list of supported schema keys doesn't look like a fun/scalable thing to do (and not only cerberus core but also subclasses should extend it). Using getattr to verify if keys are actually matched by validation functions would probably do the trick though. I am traveling around this week, can't really play with the code as I would, but feel free to give it a shot if you wish. If we can spit out an elegant/light way to do this then I'm all for it.

@ch3pjw

This comment has been minimized.

Copy link

ch3pjw commented Jun 3, 2014

Sorry I missed your message, been rather hectic of late. I've had a stab at this, see what you think. I think it basically catches all the cases that you already catch during document validation, but when you bung in the schema instead...

@ch3pjw

This comment has been minimized.

Copy link

ch3pjw commented Jun 8, 2014

@nicolaiarocci is it likely that you'll end up merging #41 in one form or another?

@nicolaiarocci

This comment has been minimized.

Copy link
Member

nicolaiarocci commented Jun 8, 2014

See my response on the PR thread (and sorry for the delayed feedback!)

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