Marshmallow Schema construction should be handled by Parser

[marshmallow-code]

Marshmallow Schema construction should be handled by Parser

[sloria]

Thanks for the suggestion, @DamianHeard . I think this is worth considering. You make good points about the API here. One alternative I can think of would be to pass a callable that takes the request. That would enable use cases such as:

def make_user_schema(request):
    only = request.query['fields']
     # Once marshmallow 2.2 is released...
    partial = request.method == 'PATCH'
    return UserSchema(only=only, partial=partial)

@use_args(make_user_schema)
def my_view(request, args):
    #...

Thoughts?

[DamianHeard]

Thanks for you're prompt reply.

So yes I think some sort of setup callable would definitely be useful. In particular, this would solve one of my up coming feature requests in advice: the ability to provide the request in the marshmallow context.

The downside to this suggestion is that it's less declarative as the schema associated with the view is now slightly harder to find in the callable. This might sound like a small thing but this is one of the things me and my co-workers enjoy most about using webargs.

Can I suggest the following iteration on you're suggestion. The callable should accept the schema class along with the request:

def  make_schema(cls, request):
    only = request.queryu['fields']
    partial = request.method == 'PATCH'
    return cls(only=only, partial=partial)


@use_args(UserSchema, callable=make_schema)
def my_view(request, args):
    # ...

This is more declarative and allows callables to be composed with schemas rather than be hard wired which should promote re-use.

I could for example do:

def use_args_with(cls, schema_args, **kwargs):
    return use_args(cls, lambda kls, request: kls(context={'request': request}, **schema_args), **kwargs)


 @use_args_with(UserSchema, schema_args={'only': ('name', 'email')})
def my_view(request, args):
    # ...

In the event a callable isn't provided I would suggest the Parser would just fall back on the schemas constructor.

[sloria]

I'm not sure adding a callable param is the most intuitive solution. What happens if a schema instance is passed? Would the callable have to handle both instances or classes? A function that returns an instance of a Schema seems more predictable.

You could still easily implement use_args_with :

def use_args_with(schema_cls, schema_kwargs=None, **kwargs):
    schema_kwargs = schema_kwargs or {}
    def maker(request):
        return schema_cls(only, partial, **schema_kwargs)
    return use_args(maker, **kwargs)

@use_args_with(UserSchema, schema_kwargs={'only': ('name', 'email')})
def my_view(request, args):

[DamianHeard]

How would you modify this approach to work when the schema is specified as a dict? Since the Parser is presently responsible for constructing the schema it wouldn't be available in the closure.

[sloria]

How would you modify this approach to work when the schema is specified as a dict?

def use_args_with(schema_cls, schema_kwargs=None, **kwargs):
    if not issubclass(schema_cls, Schema):
        schema_cls = argmap2schema(schema_cls)
    schema_kwargs = schema_kwargs or {}
    def maker(request):
        return schema_cls(only, partial, **schema_kwargs)
    return use_args(maker, **kwargs)

Edit: Fix conditional.

[DamianHeard]

Thanks for your speedy response.

While I don't love that it will certainly produce the required result. The main downside I see is that between the various parser implemention and the decorators there's already a lot of places converting dicts objects to ma.Schema and this would continue that proliferation rather than taking advantage of what's already been implemented.

I agree that in my suggestion supplying callable a schema has already been instantiated is far from clearly defined. If you recall my earlier points I was leaning towards ultimately removing support for passing in instantiated schemas for the following reasons:

• The current approach requires API users to explicitly set strict=True either in the schema constructor or the schema Meta class. This could be ensured internally if core.Parser handles construction.
• Since webargs presently requires schema instances be constructed at module scope and usage of the marshmallow context will persist between requests. Re-constructing the schema each requests ensures no state can persist.
• Presently core.Parser is already responsible for schema construction with dictionary based webargs usage. The proposed change would bring the two usages more into line.

That said I can see this might be a major departure from your thoughts and maybe entirely unpalatable. In addition to this, I don't for example have a solution of how one would easily supply constructor arguments to the schema without also using a callable or making further modifications to the decorators.

I'm still happy to put a PR together if you're happy for me to push it over the line in my own time. If I come up with an alternative approach I'll be sure to slap it up here for discussion prior to then.

[sloria]

Good points in favor of passing a Schema class over an instance. However, I'm not sure we want to completely drop support for passing instances. Passing an instance can be useful (1) when the constructor args don't need to be dynamic and (2) for performance reasons (avoiding the overhead of instantiating a Schema).

The main advantage of allowing a callable is that it enforces the mutual exclusivity of passing a dict vs. Schema class vs. Schema instance.

[sloria]

Added in f77b288