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

Extending UrlDispatcher and passing app object #1365

Closed
toumorokoshi opened this issue Nov 4, 2016 · 3 comments
Closed

Extending UrlDispatcher and passing app object #1365

toumorokoshi opened this issue Nov 4, 2016 · 3 comments
Labels

Comments

@toumorokoshi
Copy link
Contributor

Long story short

The new version of aiohttp supports subapps. Which is awesome! Unfortunately, it makes extending UrlDispatcher a little tricky.

I've previously been extending the UrlDispatcher for my extension (I add a way to mount my api methods):

https://github.com/toumorokoshi/aiohttp-transmute/blob/master/aiohttp_transmute/url_dispatcher.py#L8

Example:

https://github.com/toumorokoshi/aiohttp-transmute/blob/master/aiohttp_transmute/tests/example.py#L42

However, this breaks in the newest version, due to the new app object requirement. Unfortunately, since I have to pass the router into the app object during creation, I don't have the app object to properly instantiate the Router.

Expected behaviour

I'm thinking something like this?

app = web.Application(loop=loop, router_class=TransmuteUrlDispatcher)

Then the code for app would look something like:

    def __init__(self, *, logger=web_logger, loop=None,
                 router=None, router_class=web_urldispatcher.UrlDispatcher,
                 handler_factory=RequestHandlerFactory,
                 middlewares=(), debug=False):
        if loop is None:
            loop = asyncio.get_event_loop()
        if router is None:
            router = router_class(self)
        assert isinstance(router, AbstractRouter), router

Actual behaviour

Unable to instantiate the router properly. I'm passing in app=None right now, but that'll break for subapps.

I'm also open to alternative approaches to what I'm trying to do (mount custom routes, and then keep a dictionary that I use later for documentation).

I can do the PR for this too, if there's agreement on the approach.

Thanks!

@asvetlov
Copy link
Member

asvetlov commented Nov 6, 2016

UrlDispatcher was not designed for extending.
Use it as is -- or implement AbstractRouter from scratch.
Regarding to aiohttp-transmute I suggest another design:

  1. Push transmute context and swagger info into web.Application as app['AIOHTTP_TRANSMUTE_CONTEXT'] for example.
  2. Implement add_transmute_route(app, *args) top-level function for registering route in given application.

@toumorokoshi
Copy link
Contributor Author

ok, got it. Didn't realize that wasn't for inheriting.

I think i'll move forward with the top-level function and app values as you described. thanks!

@lock
Copy link

lock bot commented Oct 29, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants