Fuyukai / Kyoukai

[OLD] A fully async web framework for Python3.5+ using asyncio
https://mirai.veriny.tf
MIT License
298 stars 14 forks source link

Assigning hooks per route #23

Closed simonsharp closed 7 years ago

simonsharp commented 7 years ago

I've been looking at ways to assign before_request hooks to individual routes, but as far as I can tell Kyoukai only allows assigning hooks to an entire blueprint. I'm currently validating incoming request data using a before_request hook, but it's looking up the corresponding validator using a dictionary, and I would rather not do that every single time a request comes in.

As an alternative I've attempted to use decorators on routes:

def validate(func):
  def wrapped(ctx, *args):
    #validation code here (mostly for the form data)
    return await func(ctx, *args)
  return wrapped

@kyk.route('/some/endpoint/<int:id>', methods=['GET'])
@validate
async def some_endpoint(ctx: HTTPRequestContext, id: int)
  return ('a response',200,{})

but Kyoukai does some pretty strict signature checking in Kyoukai/route.py -> Route.check_route_args, so using an *args parameter doesn't seem to work. I can think of some hacky ways to get around this but I'd rather do something clean. Are there any ideas on how to approach this without doing something a little hacky?

Fuyukai commented 7 years ago

You can do the decorator thing with @functools.wraps(func), but I will add a proper way to add hooks per-route later.

simonsharp commented 7 years ago

The functools.wraps appears to work. I should have thought of that to begin with. Thanks for the help! I'll look out for the per-route hook feature.

working sample code for any future viewers (note: have to pass in keyword args rather than arg list):

import functools

def printBefore(n):
    def decorator(func):
        @functools.wraps(func)
        async def wrapped(ctx, **params):
            print(n)
            return await func(ctx, **params)
        return wrapped
    return decorator

@kyk.route('/some/endpoint/<int:id>', methods=['GET'])
@printBefore("this will print before some_endpoint executes")
async def some_endpoint(ctx: HTTPRequestContext, id: int)
    return ('a response',200,{})
Fuyukai commented 7 years ago

Pushed a new version to Git that should have these hooks natively - can you try them?

Usage:


@kyk.route("/")
async def index(ctx):
    ...

@index.before_request
async def print_before():
    print("Before request")
simonsharp commented 7 years ago
  1. Kyoukai tries to call the hook with the ctx object as an argument. As far as I can tell, it omits other arguments. I suppose you could get the url path arguments from the ctx object anyway. That's more of a framework design choice for you to make I think?
    File ".../lib/python3.6/site-packages/kyoukai/route.py", line 92, in invoke_function
    ctx = await hook(ctx)
    TypeError: printBeforeRoute() takes 0 positional arguments but 1 was given

@kai.route('/', methods=['GET']) async def some_route(ctx: HTTPRequestContext, someparameter: int): print('in the route!') return 'some response'

@some_route.before_request async def printBeforeRoute(): print('before the route')

2. What ever is returned from the hook will become the new `ctx` object. So if the route and hook are like this:
```python
@kai.route('/<int:someparameter>', methods=['GET'])
async def some_route(ctx: HTTPRequestContext, someparameter: int):
    print('in the route!')
    print(ctx)
    print(ctx.request)
    return 'some response'

@some_route.before_request
async def printBeforeRoute(ctx):
    print('before the route')

then ctx will become None:

before the route
in the route!
None

Unhandled exception in route function
Traceback (most recent call last):
  File "...", line 119, in some_route
    print(ctx.request)
AttributeError: 'NoneType' object has no attribute 'request'

again how you want to handle all of this is up to you 🐱

Fuyukai commented 7 years ago

The first one is how hooks are designed - they always take ctx as a param.

I can special case returning None from a pre/post hook, and make it use the existing context or response before, which seems more logical to me.

simonsharp commented 7 years ago

Oh I see, so ctx will be left alone unless the hook returns something to replace it with?

Fuyukai commented 7 years ago

It will, now.

Closing this as the original scope of the issue is fixed.