flask-smorest icon indicating copy to clipboard operation
flask-smorest copied to clipboard

Are query based blueprint arguments after path arguments formally unsupported?

Open palomino79 opened this issue 2 years ago • 5 comments

As an example (modified from the documentation sample code):

class PetSchema(ma.Schema):
    id = ma.fields.Int(dump_only=True)
    name = ma.fields.String()


class PetQueryArgsSchema(ma.Schema):
    name = ma.fields.String(required=False)


blp = Blueprint(
    "pets", "pets", url_prefix="/pets", description="Operations on pets"
)

@blp.route("/<pet_id>")
class PetsById(MethodView):
    @blp.arguments(PetQueryArgsSchema, location="query")
    @blp.response(200, PetSchema)
    def get(self, pet_id, query_args):
        ps = PetSchema()
        ps.id = 1
        ps.name = f"pet_id: {pet_id}, query_args: {query_args}"
        return ps

Intuitively, I would expect the PetsById.get method to populate with the pet_id as the first positional argument, followed by the query_args as a dictionary. It turns out it doesn't really work like this. In fact, it doesn't really work at all. This produces a TypeError as a result of mulitple arguments for 'pet_id'.

I found these other discussions: #219, #220, related to this issue, but I'm not sure if there were any conclusions drawn as to what the behavior here should, ideally, look like. For some context, my interest is in constructing an endpoint with a mandatory path argument, but a set of optional filtering values passed in the query. That behavior does not appear to be supported by blueprint argument decorators, however. That said, I suppose I'm ultimately asking is this either, 1) a bug in how flask-smorest handles these argument combinations, or 2) simply an unsupported feature that could perhaps be clarified by updates to the documentation?

palomino79 avatar Aug 27 '23 19:08 palomino79

Yeah, it works if you write

    def get(self, query_args, pet_id):

The order is not intuitive. I live with it but I admit it kinda sucks.

@greyli has been working on arguments order for APIFlask. I don't know if this also affected path parameters. I believe it involved overriding an underscored method, but maybe things have changed since then.

lafrech avatar Aug 28 '23 08:08 lafrech

@lafrech Thanks for the insight. That threw me off in a big way, but your example was spot on. I might try to add a note in arguments.rst to explain this, as I never would have thought to prepend the query argument dictionaries before the path argument in the get() method, unless you had let me know to do that, if that's alright.

palomino79 avatar Aug 28 '23 18:08 palomino79

Sure.

There is an example hidden in the quickstart, already, but an explicit statement in "arguments" docs wouldn't hurt.

@blp.route("/<pet_id>")
class PetsById(MethodView):

    [...]

    @blp.arguments(PetSchema)
    @blp.response(200, PetSchema)
    def put(self, update_data, pet_id):
        """Update existing pet"""

lafrech avatar Aug 29 '23 06:08 lafrech

@lafrech webargs 8.3.0 added a USE_ARGS_POSITIONAL config for the parser, you can enable this in flask-smorest to ensure the argument order (actually you don't need to care about the order since all args are in keyword argument). See the docs for more detail: https://webargs.readthedocs.io/en/latest/advanced.html#argument-passing-and-arg-name

This would be a breaking change so I released APIFlask 2.0 for this change (https://github.com/apiflask/apiflask/pull/450).

Related discussion: https://github.com/marshmallow-code/webargs/issues/830, https://github.com/marshmallow-code/webargs/pull/833

greyli avatar Aug 29 '23 13:08 greyli

Good point. I didn't take the time to try this in flask-smorest. I think I'll make extensive use of the arg_name argument as I like view function arguments to have meaningful names (pet vs. json_data).

I'm wondering if this is something that has to be forced in flask-smorest or if users may be allowed to choose. For now flask-smorest has always let the user set the value of use_kwargs.

In fact, I didn't try it but users should be able to set that parser attribute themselves without any change to flask-smorest. And since we pass kwargs transparently, they can even pass arg_name.

lafrech avatar Aug 29 '23 14:08 lafrech