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

Handling Arg(type, multiple=True, allow_missing=True) #30

Merged
merged 1 commit into from Dec 24, 2014

Conversation

@venuatu
Copy link
Contributor

commented Dec 19, 2014

core.get_value gets the _Missing type, places it into a list and then fails the type conversion/validation.
The attached test gives a 400 with: 'message': 'Expected type "integer" for ids, got "_Missing"' and only seems to be hit when there is something in the payload

A terribly quick hack is in: venuatu@7824c96 but it doesn't take allow_missing into consideration.
It looks like core.get_value might need a refactor to take allow_missing and/or required or pushing the function into the argument class.
What would be the best way of fixing this?

@sloria

This comment has been minimized.

Copy link
Member

commented Dec 23, 2014

@venuatu Thanks you for catching this and adding the failing test. I think the solution here is to allow Missing to be returned by get_value, even when multiple=True.

def get_value(d, name, multiple):
    val = d.get(name, Missing)
    if multiple and val is not Missing:
        #...
    return val

That way, the Missing value can be handled by the Parser, and required and allow_missing should behave as expected.

Will need to think more about possible edge cases, but I think this is the desired solution since it is a minimal change doesn't affect the public interface.

@sloria sloria merged commit 756ea97 into marshmallow-code:dev Dec 24, 2014

1 check failed

continuous-integration/travis-ci The Travis CI build failed
Details
@sloria

This comment has been minimized.

Copy link
Member

commented Dec 24, 2014

Confirmed that the above fix works. Merged into dev. Thanks again for reporting!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.