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

Are query based blueprint arguments after path arguments formally unsupported? #546

Open
palomino79 opened this issue Aug 27, 2023 · 5 comments

Comments

@palomino79
Copy link

palomino79 commented Aug 27, 2023

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?

@lafrech
Copy link
Member

lafrech commented Aug 28, 2023

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.

@palomino79
Copy link
Author

@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.

@lafrech
Copy link
Member

lafrech commented Aug 29, 2023

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"""

@greyli
Copy link
Contributor

greyli commented Aug 29, 2023

@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 (apiflask/apiflask#450).

Related discussion: marshmallow-code/webargs#830, marshmallow-code/webargs#833

@lafrech
Copy link
Member

lafrech commented Aug 29, 2023

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.

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