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

Use marshmallow under the hood #61

Closed
sloria opened this issue Sep 20, 2015 · 11 comments
Closed

Use marshmallow under the hood #61

sloria opened this issue Sep 20, 2015 · 11 comments

Comments

@sloria
Copy link
Member

sloria commented Sep 20, 2015

Proposal

Replace Arg objects with marshmallow fields. Use marshmallow for validation.

Why?

Webargs and marshmallow have redundant functionality for validating input data against a user-defined schema. Marshmallow does a better job of than webargs in a number of respects:

Proposed API

from webargs import use_kwargs, fields


@use_kwargs({
    'title': fields.Str(required=True, location='json'),
    'description': fields.Str(required=False),
})
def myview(title, description):
    return 'title: {}, description: {}'.format(title, description)


# with nesting
@use_kwargs({
    'data': fields.Nested({
        'title': fields.Str(),
        'description': fields.Str(),
    })
})
def myview(data):
    return 'title: {}, description: {}'.format(**data)


# respects load_from
@use_kwargs({
    'x_custom_header': fields.Str(load_from='X-Custom-Header')
})
def myview(x_custom_header):
    return 'Header: {}'.format(x_custom_header)


# Passing in a Schema
from marshmallow import Schema

class PostSchema(Schema):
    title = fields.Str(required=True, location='json')
    description = fields.Str(required=False)
    id = fields.Int(dump_only=True)  # won't be included in parsed output

@use_kwargs(PostSchema)
def myview(title, description):
    return 'title: {}, description: {}'.format(title, description)
@marcellarius
Copy link
Contributor

This seems like a good solution to me.

On Mon, Sep 21, 2015 at 9:26 AM, Steven Loria notifications@github.com
wrote:

Proposal

Replace Arg objects with marshmallow fields. Use marshmallow for
validation.
Why?

Webargs and marshmallow have redundant functionality for validating input
data against a user-defined schema. Marshmallow does a better job of than
webargs in a number of respects:

Proposed API

from webargs import use_kwargs, fields

@use_kwargs({
'title': fields.Str(required=True, location='json'),
'description': fields.Str(required=False),
})def myview(title, description):
return 'title: {}, description: {}'.format(title, description)

with nesting@use_kwargs({

'data': fields.Nested({
    'title': fields.Str(),
    'description': fields.Str(),
})

})def myview(data):
return 'title: {}, description: {}'.format(**data)

respects load_from@use_kwargs({

'x_custom_header': fields.Str(load_from='X-Custom-Header')

})def myview(x_custom_header):
return 'Header: {}'.format(x_custom_header)

Passing in a Schemafrom marshmallow import Schema

class PostSchema(Schema):
title = fields.Str(required=True, location='json')
description = fields.Str(required=False)
id = fields.Int(dump_only=True) # won't be included in parsed output
@use_kwargs(PostSchema)def myview(title, description):
return 'title: {}, description: {}'.format(title, description)


Reply to this email directly or view it on GitHub
#61.

@hello-josh
Copy link
Contributor

+1

@sloria
Copy link
Member Author

sloria commented Sep 22, 2015

Making lots of progress on this.

I haven't yet decided where fields should be imported from.

Here are the alternatives (note that Nested is a webargs-specific field):

(1) Import from webargs.fields

from webargs import fields
from webargs.flaskparser import use_args
from marshmallow import validate

@use_args({
    'data': fields.Nested({
        'id': fields.Int(),
        'email': fields.Str(validate=validate.Email()),
    })
})

(2) Import from webargs

import webargs as wa
from webargs.flaskparser import use_args
from marshmallow import validate

@use_args({
    'data': wa.Nested({
        'id': wa.Int(),
        'email': wa.Str(validate=validate.Email()),
    })
})

(3) Import from marshmallow

from webargs import Nested  # or from webargs.fields import Nested
from webargs.flaskparser import use_args
from marshmallow import fields, validate

@use_args({
    'data': Nested({
        'id': fields.Int(),
        'email': fields.Str(validate=validate.Email()),
    })
})

Any preferences?

@marcellarius
Copy link
Contributor

  1. or 3). I think option 3) makes it clear that it's using marshmallow.

On Tue, Sep 22, 2015 at 2:51 PM, Steven Loria notifications@github.com
wrote:

Making lots of progress on this.

I haven't yet decided where fields should be imported from.

Here are the alternatives (note that Nested is a webargs-specific field):

(1) Import from webargs.fields

from webargs import fieldsfrom webargs.flaskparser import use_argsfrom marshmallow import validate
@use_args({
'data': fields.Nested({
'id': fields.Int(),
'email': fields.Str(validate=validate.Email()),
})
})

(2) Import from webargs

import webargs as wafrom webargs.flaskparser import use_argsfrom marshmallow import validate
@use_args({
'data': wa.Nested({
'id': wa.Int(),
'email': wa.Str(validate=validate.Email()),
})
})

(3) Import from marshmallow

from webargs import Nestedfrom webargs.flaskparser import use_argsfrom marshmallow import fields, validate
@use_args({
'data': Nested({
'id': fields.Int(),
'email': fields.Str(validate=validate.Email()),
})
})

Any preferences?


Reply to this email directly or view it on GitHub
#61 (comment).

@sloria
Copy link
Member Author

sloria commented Sep 22, 2015

Fair point. On the other hand, perhaps we don't want to "leak" the marshmallow abstraction? As a user, I don't really like being required to import a 3rd-party library's (marshmallow) modules in the common case, but that's personal preference.

@sloria
Copy link
Member Author

sloria commented Sep 22, 2015

From discussion with @jmcarp : "I think i prefer 1 or 2 just so it's harder to use the wrong Nested".

@hello-josh
Copy link
Contributor

I would argue that you shouldn't import from Marshmallow at all. If you want to use Marshmallow under the good, go all the way.

from webargs import fields, validate

@sloria
Copy link
Member Author

sloria commented Sep 22, 2015

To play devil's advocate: there's a risk of hiding too much. If users import Schema from webargs, they might think that webargs modifies behavior of vanilla marshmallow Schemas in some way.

That said, I think importing fields and validate from webargs is fine.

@sloria
Copy link
Member Author

sloria commented Sep 22, 2015

There's also an argument for future-proofing: we can always add imports to webargs without breaking backwards-compat; we cannot remove them without breaking compat.

sloria added a commit that referenced this issue Sep 25, 2015
sloria added a commit that referenced this issue Sep 25, 2015
@sloria
Copy link
Member Author

sloria commented Sep 27, 2015

The marshmallow branch is merged. We'll have a release out later today.

@sloria sloria closed this as completed Sep 27, 2015
@hello-josh
Copy link
Contributor

👍

sloria added a commit to CenterForOpenScience/django-elasticsearch-metrics that referenced this issue Aug 20, 2018
This adds a custom `Date` field which respects
`settings.TIMEZONE`.

This also exposes elasticsearch_dsl's fields from the top-level
module, so that all fields are imported from the same place:

```python
from elasticsearch_metrics import Keyword, Date
```

This approach was the same used in webargs, which uses
marshmallow fields under the hood but overrides a single
field. Discussion: marshmallow-code/webargs#61 (comment)

close #18
sloria added a commit to CenterForOpenScience/django-elasticsearch-metrics that referenced this issue Aug 20, 2018
This adds a custom `Date` field which respects
`settings.TIMEZONE`.

This also exposes elasticsearch_dsl's fields from the top-level
module, so that all fields are imported from the same place:

```python
from elasticsearch_metrics import Keyword, Date
```

This approach was the same used in webargs, which uses
marshmallow fields under the hood but overrides a single
field. Discussion: marshmallow-code/webargs#61 (comment)

close #18
sloria added a commit to CenterForOpenScience/django-elasticsearch-metrics that referenced this issue Aug 20, 2018
This adds a custom `Date` field which respects
`settings.TIMEZONE`.

This also exposes elasticsearch_dsl's fields from the top-level
module, so that all fields are imported from the same place:

```python
from elasticsearch_metrics import Keyword, Date
```

This approach was the same used in webargs, which uses
marshmallow fields under the hood but overrides a single
field. Discussion: marshmallow-code/webargs#61 (comment)

close #18
sloria added a commit to CenterForOpenScience/django-elasticsearch-metrics that referenced this issue Aug 20, 2018
This adds a custom `Date` field which respects
`settings.TIMEZONE`.

This also exposes elasticsearch_dsl's fields from the metrics
module, so that all fields are imported from the same place:

```python
from elasticsearch_metrics import Keyword, Date
```

This approach was the same used in webargs, which uses
marshmallow fields under the hood but overrides a single
field. Discussion: marshmallow-code/webargs#61 (comment)

close #18
sloria added a commit to CenterForOpenScience/django-elasticsearch-metrics that referenced this issue Aug 21, 2018
This adds a custom `Date` field which respects
`settings.TIMEZONE`.

This also exposes elasticsearch_dsl's fields from the metrics
module, so that all fields are imported from the same place:

```python
from elasticsearch_metrics import Keyword, Date
```

This approach was the same used in webargs, which uses
marshmallow fields under the hood but overrides a single
field. Discussion: marshmallow-code/webargs#61 (comment)

close #18
sloria added a commit to CenterForOpenScience/django-elasticsearch-metrics that referenced this issue Aug 21, 2018
This adds a custom `Date` field which respects
`settings.TIMEZONE`.

This also exposes elasticsearch_dsl's fields from the metrics
module, so that all fields are imported from the same place:

```python
from elasticsearch_metrics import metrics

class MyMetric(metrics.Metric):
  my_keyword = metrics.Keyword()
  my_date = metrics.Date()
```

This approach was the same used in webargs, which uses
marshmallow fields under the hood but overrides a single
field. Discussion: marshmallow-code/webargs#61 (comment)

close #18
sloria added a commit to CenterForOpenScience/django-elasticsearch-metrics that referenced this issue Aug 21, 2018
This adds a custom `Date` field which respects
`settings.TIMEZONE`.

This also exposes elasticsearch_dsl's fields from the metrics
module, so that all fields are imported from the same place:

```python
from elasticsearch_metrics import metrics

class MyMetric(metrics.Metric):
  my_keyword = metrics.Keyword()
  my_date = metrics.Date()
```

This approach was the same used in webargs, which uses
marshmallow fields under the hood but overrides a single
field. Discussion: marshmallow-code/webargs#61 (comment)

close #18
sloria added a commit to CenterForOpenScience/django-elasticsearch-metrics that referenced this issue Aug 21, 2018
This adds a custom `Date` field which respects
`settings.TIMEZONE`.

This also exposes elasticsearch_dsl's fields from the metrics
module, so that all fields are imported from the same place:

```python
from elasticsearch_metrics import metrics

class MyMetric(metrics.Metric):
  my_keyword = metrics.Keyword()
  my_date = metrics.Date()
```

This approach was the same used in webargs, which uses
marshmallow fields under the hood but overrides a single
field. Discussion: marshmallow-code/webargs#61 (comment)

close #18
sloria added a commit to CenterForOpenScience/django-elasticsearch-metrics that referenced this issue Aug 21, 2018
This adds a custom `Date` field which respects
`settings.TIMEZONE`.

This also exposes elasticsearch_dsl's fields from the metrics
module, so that all fields are imported from the same place:

```python
from elasticsearch_metrics import metrics

class MyMetric(metrics.Metric):
  my_keyword = metrics.Keyword()
  my_date = metrics.Date()
```

This approach was the same used in webargs, which uses
marshmallow fields under the hood but overrides a single
field. Discussion: marshmallow-code/webargs#61 (comment)

close #18
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