adriangb / xpresso

A composable Python ASGI web framework
https://xpresso-api.dev/
MIT License
178 stars 4 forks source link

feat: add @Router.get, @Router.put, etc. decorators to Router #50

Closed adriangb closed 2 years ago

adriangb commented 2 years ago

Background: https://github.com/encode/starlette/pull/704

There are pros and cons to the decorator / list of routes approach. I think the big pro of the decorator approach is having the path parameters close to the endpoint function. The main con is that it introduces a lot of code paths, for example with App.add_middleware() and the relationship between App and App.router.

A good compromise may be to add decorators only for Path. This gives us the best of both worlds:

The main issue is how to deal with Operation. I guess we'd have to make the decorators accept all of the parameters of Operation, which I don't like.

adriangb commented 2 years ago

Hmm realizing we would have to add these to Router, not Path.

It gets super tricky then because you'd have to be able to pass parameters 2 levels deep (e.g. to get something to Operation). Or you mash up all parameters into one. But I feel like this really deteriorates what are now clearly delineated concepts of Path and Operation which map 1:1 with OpenAPI and make it super clear what's going on.

So maybe this isn't such a good idea after all. If it was simpler, I would like to have the path parameter template closer to the endpoint that uses them. But not if it introduces a lot of rough edges and makes it harder to grasp the concepts of the framework.

adriangb commented 2 years ago

Two counterarguments to the arguments for having the decorators are:

  1. You should be able to catch name discrepancies in tests, unless you literally don't have any tests calling that endpoint.
  2. Both in Xpresso and FastAPI you can have a dependency that uses a path parameter. And at that point all bets are off the table since that dependency could be in a different file and whatnot.
  3. If yo use decorators and have multiple methods with different endpoint functions, you now have to keep the decorators in sync, e.g. if you update the path param name (this is not a problem in Xpresso currently since all of the common info goes into Path)
adriangb commented 2 years ago

This is going to have to be a punt. There are other priorities, like figuring out the Security story