squeaky-pl / japronto

Screaming-fast Python 3.5+ HTTP toolkit integrated with pipelining HTTP server based on uvloop and picohttpparser.
MIT License
8.61k stars 581 forks source link

Proposal: Route decorators #5

Open agalera opened 7 years ago

agalera commented 7 years ago

What do you think of adding decorators to the methods?

https://gist.github.com/kianxineki/81d81f5931691df83440f6103c33a600

from japronto import Application

app = Application()

# GET decorator
def get(url):
    def wrap_function(func):
        app.router.add_route(url, func, method='GET')
    return wrap_function

@get('/')
def hello(request):
    return request.Response(text='Hello world!')

app.run()
luizberti commented 7 years ago

I like this idea but the way Flask does it. The way you are proposing would just lead to things like

@get(...)
@post(...)
@put(...)
@delete(...)
...
def handler(request):
   ...

rather than

@route('/foo', methods=['get', 'post', 'put', 'delete', ...])
def handler(request):
    ...

Not a great way to route your application.

That being said routes provide a much better visibility for the API and are definitely worth implementing.

agalera commented 7 years ago
from japronto import Application

app = Application()

def route(url, *args, **kwargs):
     def wrap_function(func):
         app.router.add_route(url, func, *args, **kwargs)
         return fn
     return wrap_function

# one method
@route('/test1', method='GET')
def test1(request):
    return request.Response(text="test1")

# multiple methods
@route('/test2', methods=['GET', 'POST'])
def test2(request):
    return request.Response(text="test2")
luizberti commented 7 years ago

Your new idea still has a problem: It has two arguments with nearly identical names that do the exact same thing, only to keep the developer from making a single-element list. (in fact I only noticed this the 4th time I read the code)

I think something like this would be better

def route(url, methods=['GET',], *args, **kwargs):
    def decorator(fn):
        for method in methods:
            app.router.add_route(url, fn, method=method.upper(), *args, **kwargs)
        return fn
    return decorator

@app.route('/foo')
def handler(...) ...

Fixes the shortcomings and has the added benefit of defaulting to only allowing GET to the URL, which is a saner default than allowing for all methods. And maybe we could look at adding the ability to overload routed paths by method for example:

@app.route('/foo')
def called_when_method_is_get(request): ...

# same url
@app.route('/foo', methods=['POST', 'PUT'])
def called_when_method_is_post_or_put(request): ...

Sanic recently did this and it's a great way to parametrize the application in my opinion.

If this gets implemented, we should almost certainly make it a method from Application too.

agalera commented 7 years ago

@luizberti Yes, I think it's better if it's a GET by default. :+1:

channelcat commented 7 years ago

@get(...) @post(...) @put(...) @delete(...) ... def handler(request): ... Not a great way to route your application.

To argue for shorthand routes, we recently implemented these in Sanic, but namespaced through app, looking like this:

@app.get('/foo')
@app.post('/foo')
@app.put('/foo')
@app.delete('/foo')

I can say working with these, the code is much more readable and clean. Though it removes the possibility of using those generic method names for other purposes, I think the trade-off here is worth it, and it hasn't seemed to cause confusion as of yet.

luizberti commented 7 years ago

There is nothing prohibitive about implementing both (except maybe if you wanna get pedantic the Zen of Python saying there should be only one obvious way to do something), but I do think this is not a very good idiom if used this way. It might be useful if you wanna lock down a route or handler to a specific method and use only one of them instead of stacking them up e.g.

@app.delete('/rest_resource')
def resource_delete(request): ...

But it's bad to start stacking them up like that, especially when you start adding in CORS decorators, CSRF Token decorators, Caching decorators, etc. All of the sudden and at the blink of an eye your function signature is sandwiched between a big stack of decorators, and a big docstring. 😛

Misuse of this can easily hamper readability, and it also forces people to start adding _delete suffixes and such which makes your Python read like C kind of. Besides, no one enjoys scrolling through excruciatingly verbose code.

All in all, I'm just playing devil's advocate here. Like I said, both can be easily implemented and probably should to some extent. But I would prioritize the generic @app.route over the other ones as it is less "misuse prone". If there is any take away from this, maybe it should be that proper usage of @app.method should be clearly stated in the docs, to avoid people littering decorators everywhere.

agalera commented 7 years ago

It would be interesting to add plugins as does bottle, not to need multiple decorators, and also have control before and after each request. I give you an example of my current project

auth argument using https://github.com/kianxineki/bottlejwt

method get using bottle.default_app, its posible using explicit app

@get('/media/<_id>', auth=1, cache=True, cors=True)
def get_item(auth, _id):
    ...
squeaky-pl commented 7 years ago

Hi, I agree that this indeed would help. I must warn you though that I plan to completely change how routing is done (to cover things like blueprints and middleware but with a twist) so I don't want to include such helpers yet to alleviate maintenance cost. This is not to discourage you from doing it. Such things can be easily maintained in your own projects. Keep discussing and I will get back to you once I am done with advanced router design.

amsb commented 7 years ago

@squeaky-pl I'd personally love to see the project evolve into a next-generation Werkzeug-like core library (or even separate routing and http libraries like chi or httprouter + fasthttp in Go). With that approach, people can use the core library to build web-application frameworks satisfying diverse or specific needs and preferences while pooling resources on the common core functionality which makes them all great. Route decorators are a good example of an API that is easy to build but subjective and likely driven by project needs.

squeaky-pl commented 7 years ago

@amsb That's one of the directions I am considering as well. I would like to have performant components that can be used together or apart. On C level this can be achieved with exposing C APIs through for example capsules as well as Python-facing APIs. That's what Japronto does already for unit testing. When components are tested apart they are tested with pytest/hypothesis through Python APIs and when they are glued together they just speak through C bypassing Python facing APIs.

Martmists-GH commented 7 years ago

How much progress has been made on this so far?

gustavorps commented 7 years ago

+1

yohanboniface commented 6 years ago

@squeaky-pl you may consider https://github.com/pyrates/autoroutes, in the direction pointed by @amsb: pico modules that do one thing right, and let every one build their own stack picking the modules that fit their needs.