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

Always HTTP 400 if marshmallow==3.0.0b13 #492

Closed
wrkhenddher opened this issue Sep 13, 2018 · 3 comments
Closed

Always HTTP 400 if marshmallow==3.0.0b13 #492

wrkhenddher opened this issue Sep 13, 2018 · 3 comments

Comments

@wrkhenddher
Copy link

wrkhenddher commented Sep 13, 2018

Hi,

Just ran across this:

>               "Bad response: %s (not %s)\n%s", res_status, status, res)
E           webtest.app.AppError: Bad response: 400 Bad Request (not 200)
E           b'{"status": "error", "errors": [{"location": "cookies", "name": "cookies", "description": ["Unknown field."]}, {"location": "header", "name": "header", "description": ["Unknown field."]}, {"location": "method", "name": "method", "description": ["Unknown field."]}, {"location": "url", "name": "url", "description": ["Unknown field."]}, {"location": "path", "name": "path", "description": ["Unknown field."]}, {"location": "querystring", "name": "querystring", "description": ["Unknown field."]}]}'

../../../../.pyenv/versions/3.6.3/envs/p2server_py36/lib/python3.6/site-packages/webtest/app.py:686: AppError

The reason is marshmallow 3.0.0b13 marshmallow-code/marshmallow#911

Perhaps unknown = marshmallow.EXCLUDE is now needed here:

class Meta(object):
strict = True
ordered = True

The catch is that marshmallow wants to be more strict (than before) so relaxing it seems questionable.

Up to marshmallow 3.0.0b12, it all worked well. In 3.0.0b13, the unknown=EXCLUDE fixes the issue.

@calopez
Copy link

calopez commented Sep 17, 2018

You all right @wjehenddher, it makes sense, I tested your suggestioh and it worked as expected, however, I wonder why I can't simply use the unknown=EXCLUDE in the class Meta inside my own schema definition.
The way it is now, the change must be done in cornice/validators/_marshmallow.py
:|

@wrkhenddher
Copy link
Author

... I wonder why I can't simply use the unknown=EXCLUDE in the class Meta inside my own schema definition.
The way it is now, the change must be done in cornice/validators/_marshmallow.py

That's indeed the bummer, @calopez. Your "custom schema" sits on top of cornice's schema that deals with body (marshmallow_body_validator). The other schemas cornice creates for headers, querystring, etc. need to be configured as well.

I.e, cornice creates multiple schemas to handle all parts of the request and delegates onto "custom schemas" depending on which marshmallow_XXX_validator one uses. That's my understanding at least. Will this qualify cornice as too strongly opinionated in this case? That's a philosophical argument.

Perhaps a configuration setting in cornice/validators/_marshmallow.py that allows implementors to override those schemas might be sufficient.

@ergo, @leplatrem?

@ergo
Copy link
Collaborator

ergo commented Oct 5, 2018

This should be fixed in PR #495 .

I've reintroduced compatibility across both versions - in my opinion we should fall back to unknown=EXCLUDE again and cornice should drop the keys that are not explictly enabled.

I think we could also add an option to override meta from schema Meta - but I'm not 100% sure what potential implications that could have so I didn't add that functionality.

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

3 participants