Skip to content

Conversation

@pgjones
Copy link
Member

@pgjones pgjones commented Feb 14, 2021

I know we briefly discussed this, but I'd like to make the case with a concrete implementation. My argument is that this is actually quite a nice API and requires little implementation (especially now with the nice Scaffold). There is also precedent, in my view, that the test client also has this API. (Also, everyone copies Flask so lets copy them :p).

Commit message

This takes a popular API whereby instead of passing the HTTP method as
an argument to route it is instead used as the method name i.e.

@app.route("/", methods=["POST"])

is now writeable as,

@app.post("/")

This is simply syntatic sugar, it doesn't do anything else, but makes
it slightly easier for users.

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

@pgjones pgjones force-pushed the sugar branch 2 times, most recently from dd8c2de to 5caf74a Compare February 14, 2021 11:20
@ThiefMaster
Copy link
Member

ThiefMaster commented Feb 14, 2021

What I dislike with this approach is that - at least when you aren't writing a RESTful API - there is a very good chance that you need endpoints handling both GET and POST (render form if not submitted or validation failed, otherwise do stuff). In fact, in a classic webapp with lots of forms it's probably the most common thing after GET endpoints...

However, this case is not covered by the shortcuts and I can't think of any decent name for a method that does cover it... And I think beginners may think mixed-method routes are discouraged because of this, making them do weird stuff with the session for the basic web form usecase described above...

@pgjones
Copy link
Member Author

pgjones commented Feb 14, 2021

I believe the test failures are unrelated.

I think the existing route method covers the GET & POST case you describe really well. There is (and will still be) a large corpus of tutorials and documentation for beginners that uses route, so I don't think we need worry about them going any further astray. As a counter I think it will make it easier for beginners writing RESTful APIs with this syntax.

This takes a popular API whereby instead of passing the HTTP method as
an argument to route it is instead used as the method name i.e.

    @app.route("/", methods=["POST"])

is now writeable as,

    @app.post("/")

This is simply syntatic sugar, it doesn't do anything else, but makes
it slightly easier for users.

I've included all the methods that are relevant and aren't auto
generated i.e. not connect, head, options, and trace.
@davidism
Copy link
Member

davidism commented Mar 8, 2021

Refactored to avoid repeating the error message in each method. Changed to a TypeError since that's what's used for issues with arguments. Added tests for all methods and for the error raised.

@davidism davidism merged commit 5fea7ca into pallets:master Mar 8, 2021
@davidism davidism added this to the 2.0.0 milestone Mar 8, 2021
@greyli
Copy link
Contributor

greyli commented Mar 9, 2021

Glad to see this finally merged! After I proposed this idea and was declined in the first pallets meeting, I started to make the REST API extension I'm developing to become a framework so that I can inherit the Flask class and add these route shortcuts, now I may consider making it back to an extension...

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants