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

Default values for webargs #32

Closed
jmcarp opened this issue Oct 15, 2015 · 3 comments

Comments

@jmcarp
Copy link
Contributor

commented Oct 15, 2015

Now that webargs is using marshmallow internally, we specify default argument values using the missing parameter:

arg = fields.Str(missing='foo')

But apispec doesn't check the missing key when introspective parameter defaults--it only checks default. To get the correct swagger default, we have to specify both parameters:

arg = fields.Str(missing='foo', default='foo')

Which isn't ideal.

Since we're using the same logic to introspect fields and schemas that are used for serialization and deserialization, one approach would be to tell field2property and related methods whether they're introspecting load or dump. For the specific case of parameter defaults, we'd check default for dump and missing for load.

@sloria

This comment has been minimized.

Copy link
Member

commented Oct 16, 2015

I think the proposed approach--specifying whether we are dumping/loading--makes sense.

jmcarp added a commit to jmcarp/smore that referenced this issue Oct 16, 2015

Add dump flag.
When `dump` is `True` (the default value), introspect serialization
behavior; else introspect deserialization behavior. This is useful for
introspecting webargs fields and schemas, for which the default value
should be populated by `field.missing` rather than the usual
`field.default`.

[Resolves marshmallow-code#32]
@jmcarp jmcarp referenced this issue Oct 16, 2015

@sloria sloria closed this in #33 Oct 17, 2015

@lafrech

This comment has been minimized.

Copy link
Member

commented Apr 5, 2018

I don't have the whole webargs history in mind, maybe this was correct back then, but I think current behaviour is partly wrong.

Here's what OpenAPI docs say about default parameters:

The default value is the one that the server uses if the client does not supply the parameter value in the request.

We should use the missing attribute in any case. The default attribute of a marshmallow field is internal detail the client shouldn't care about, I suppose.

Note that if used in a REST API, the result is pretty much the same from user perspective. If using missing, the default value is added at deserialization, saved in DB and returned. If using default, the default value is not saved in DB but added at serialization and returned. To me it makes more sense to use missing, in this case.

Anyway, when adding a schema definition, I think we should use missing. I prepared a PR for that.

I was about to remove the dump parameter but it is also used to manage dump_only/load_only attribute so extra care should be taken about this. Postponing.

@sloria

This comment has been minimized.

Copy link
Member

commented Apr 10, 2018

I hadn't thought about it that way, but you're correct--missing is the correct thing to check when introspecting fields.

@sloria sloria closed this Apr 10, 2018

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