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

schema validation requires MappingSchema #307

Closed
tisdall opened this issue Jun 10, 2015 · 4 comments · Fixed by #309
Closed

schema validation requires MappingSchema #307

tisdall opened this issue Jun 10, 2015 · 4 comments · Fixed by #309

Comments

@tisdall
Copy link
Contributor

tisdall commented Jun 10, 2015

This is related to this posting on SO: http://stackoverflow.com/questions/30736627/cornice-schema-validation-with-colanderalchemy/30761861#30761861

Essentially validation will only work if the colander schema passed in is a direct subclass of MappingSchema. Using something like SchemaNode(typ=Mapping) should work just fine, but instead it throws cornice.schemas.SchemaError: schema is not a MappingSchema.

@almet
Copy link
Contributor

almet commented Jun 10, 2015

Yeah, we definitely should do some duck typing here rather than trying to do a type check. I'm not sure from the thread on Stack Overflow: do you want to work on a fix?

@tisdall
Copy link
Contributor Author

tisdall commented Jun 10, 2015

If you have time, I'd prefer it if you did. I'm a little swamped. I'm also just starting to figure out how Cornice works. ^_^

I'm going to fix ColanderAlchemy so it implements schema_type, but Cornice should probably be using typ as schema_type isn't really supposed to be used like that. I filed an issue at Pylons/colander#229 as that really is a hole waiting for other people to fall in.

@tisdall
Copy link
Contributor Author

tisdall commented Jun 10, 2015

Okay, on second thought, I'm not going to make the change to ColanderAlchemy. The schema_type() is a static method so it has no access to the actual instance. When you use ColanderAlchemy it assigns a value to unknown of Mapping and schema_type won't ever see that. I think I could make it so schema_type() is no longer a static method, but then it's getting rather hackish.

I'll put together a PR to properly use the typ instead of schema_type() in Cornice.

@tisdall
Copy link
Contributor Author

tisdall commented Jul 3, 2015

I'm going to close this issue as PR #309 fixes this issue.

@tisdall tisdall closed this as completed Jul 3, 2015
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 a pull request may close this issue.

2 participants