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

Consider using kwargs instead of args in route invocation #429

Closed
jamesls opened this issue Jul 26, 2017 · 5 comments
Closed

Consider using kwargs instead of args in route invocation #429

jamesls opened this issue Jul 26, 2017 · 5 comments

Comments

@jamesls
Copy link
Member

jamesls commented Jul 26, 2017

We use *args when we invoke a view function, whereas **kwargs seems to make more sense (this is what Flask does).

This is technically a breaking change, but will only affect users that don't match their function argument names with the capture group names:

# Continues to work
@app.route('/foo/{bar}')
def foo(bar): pass

# Would now be broken
@app.route('/foo/{bar}')
def foo(notbar): pass

Not sure if it's worth the breaking change, but we should decide if we want to do this now if we want to change it.

Let me know if anyone feels strongly either way.

@kyleknap
Copy link
Contributor

kyleknap commented Jul 26, 2017

What is the benefits of using **kwargs over *args? Mainly interested in why flask would do it. Any ideas on why that would be the case?

Just thinking of benefits:

  1. You do not have to worry about the order of the path variable in the route decorator when I define my function. For example, I could do:
@app.route('{user}/{game_id}')
def get_game(game_id, user):
     pass
  1. I can use defaults if I am executing a handler somewhere else? Not sure why you would do this as it is probably better to define a wrapper around it.

The main downside though of course is that your handler parameters are now tied to the names of the parameters provided in the route decorator. So it is a bit more work if you change the name of one of the route decorator path variables as you will have to change it in the handler as well.

Currently, I do not really have any strong feelings about it.

@jamesls
Copy link
Member Author

jamesls commented Jul 26, 2017

Yeah so the feedback was exactly your code snippet. In this case, the expectation for the user was that the {game_id} would match up to the game_id param, and not be based on the param order. It was surprising that the game_id function arg maps to the {user} url param.

We could also give slightly better validation errors if we wanted to validate that for every capture group, you have a corresponding function argument with the same name (with the appropriate edge cases handled for **kwargs and py3 kw only args).

Oh and other benefit is that this is how Flask does it (uses kwargs), so someone who' familiar with Flask might be surprised that we do it slightly different.

@kyleknap
Copy link
Contributor

kyleknap commented Jul 26, 2017

Right. Better validation is something you would also get. I think the **kwargs approach is growing on me (i.e. if I were to do this from scratch, I would most likely pick **kwargs over *args).

@jamesls
Copy link
Member Author

jamesls commented Jul 26, 2017

Yeah FWIW I arbitrarily chose *args in the initial implementation, I didn't really have a strong reason to do it that way and I didn't remember offhand what Flask did.

@kyleknap
Copy link
Contributor

Alright. I think I would vote to make it **kwargs. It may break some users, but it is generally documented to match the names of path parameters with the names of the handler parameters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants