aio-libs / aiohttp

Asynchronous HTTP client/server framework for asyncio and Python
https://docs.aiohttp.org
Other
15.14k stars 2.02k forks source link

add chaining / mounting routing tables #2923

Open FlorianLudwig opened 6 years ago

FlorianLudwig commented 6 years ago

Long story short

module = web.RouteTableDef()

@module.get('/status')
async def status(handler):
    return web.Response(text="I am fine.")

root = web.RouteTableDef()
root.mount("/api/v1", api_module)

Now we got /api/v1/status as a route.

Implementation

If there is interest in having this feature I am willing to look into implementing it and submitting a pull request. I just would like to first clarify API definitions.

asvetlov commented 6 years ago

I'm curious why you cannot use full paths (with prefix) in decorators?

FlorianLudwig commented 6 years ago

For me this is mostly interesting when having multiple python modules.

If I only got one RouteTableDef it is one singleton every module got to import to register the routes. I prefer to have it the other way around: one module that imports all my modules and stitches those together.

It also helps creating reusable modules.

hubo1016 commented 6 years ago

Looks interesting. Should the routing table be mounted statically (paths are copied and imported, further modification on the routing table does not affect the root table), or dynamically (requests are redirected to the routing table so further modification on the mounted routing table take effect)

asvetlov commented 6 years ago

I recall an alternative proposal: adding prefix param to RouteTableDef: router = RoutTableDef(prefix='/api/v1').

I have no preference and not sure if the feature is needed at all. @aio-libs/committers please share your opinions.

popravich commented 6 years ago

@aio-libs/aiohttp-committers

PS: I think these all teams should be reorganized.

asvetlov commented 6 years ago

@popravich please make an issue for discussion

kxepal commented 6 years ago

-0. To build good reusable module you don't need to have any aiohttp (routing) references there. Just a library that could be reusable. How it could be routed, with cors or without it's all about the final user and their app. I think this would only add one more abstraction layer for routing and makes it more hard to understand since you need not just find a route definition itself, but the parent routes which defines a prefix. Which could be multiple ones (/api/v1, /api/v2, etc.).

samuelcolvin commented 6 years ago

I would agree with @kxepal that there's no need to add anything further. The current options are powerful / flexible / confusing enough.

updated, see below.

samuelcolvin commented 6 years ago

Sorry, I've thought about this and changed my opinion.

As with in django it would be useful for big projects to be able to add a RouteTableDef like you can currently add a view.

The alternative of module = web.RouteTableDef(prefix='foo/bar') would also work, but I think would be less clear.

kxepal commented 6 years ago

For big projects RouteTableDef wouldn't work without painkillers. You don't want to have your routes get spread across the project while you do want to separate routes from views and views from controllers / models. This would require quite a different approach which will go in conflict with decorator routes style in anyway.

IIRC, this feature was introduced to make simple projects simple and flasky, but not for something bigger.

samuelcolvin commented 6 years ago

ok makes sense, I still think it would be useful to add a prefix option to add_routes or similar.

Currently I think the only way to add a collection of routes with a prefix is with add_subapp which is perhaps more complicated than it needs to be.

Basically I'm saying it would be useful to have an equivalent of path('credit/', include(extra_patterns)), I'm not that fussed about flasky or not flasky, but it would make sense that this could accept either a list of RouteDef items or RouteTableDef.

FlorianLudwig commented 6 years ago

Maybe a little back story: While building a growing number of (micro) services we find ourselves creating reusable blocks to construct them. Including routing. While moving towards python 3.6 and aiohttp we felt something missing. The solution posted above worked well for us where we came from but I don't claim it is the right solution nor the aiohttp-way ;)

add_subapp does not work in our use cases since then we have different contexts.

The "routing should not be part of a reusable library" also doesn't work for me because the routing is not that small part of the library and just accepting it as non-reusable is not an option :] Our use cases are building api services. So we have very homogeneous requirements regarding authentication etc. so it might be easier to reuse routing code then in more classic web development.

asvetlov commented 6 years ago

Well, looks like mount() is a viable improvement. Before going future we need to answer the question:

Should the routing table be mounted statically (paths are copied and imported, further modification on the routing table does not affect the root table), or dynamically (requests are redirected to the routing table so further modification on the mounted routing table take effect)

I think the first option is easier for implementing and understanding.

asvetlov commented 6 years ago

Also, we need web.mount() with the similar functionality maybe. BTW mount() or include()?

samuelcolvin commented 6 years ago

why not just add a prefix argument to add_routes? that avoids add another fuction to an already complicated part of aiohttp?

asvetlov commented 6 years ago

@FlorianLudwig ? ^^^

hubo1016 commented 6 years ago

@samuelcolvin I think the point of this feature request is to allow adding routes from a routing table to another routing table. A very large project may contain a lot of sub modules, each may have their own sub modules to form a sub module tree. The root project does not know - and should not know about all the descendants, but only the direct children. So if a sub module can "import" the routing table of its own sub module, it makes creating the root routing table easier.

samuelcolvin commented 6 years ago

From above

While building a growing number of (micro) services we find ourselves creating reusable blocks to construct them. Including routing.

No "very large project" mentioned.

In this scenario app.router.add_routes(module, prefix="/api/v1") would suffice as the micro-service could import the reusable blocks and add/mount/include them under the given prefix.

I'm not saying mount() isn't required, just asking whether another argument in add_routes() would suffice.