litestar-org / litestar

Production-ready, Light, Flexible and Extensible ASGI API framework | Effortlessly Build Performant APIs
https://litestar.dev/
MIT License
5.64k stars 382 forks source link

Question: How to mount an app instance? #241

Closed mybigman closed 2 years ago

mybigman commented 2 years ago

Is this possible? If not could it be :)

admin_app = Starlette(
    routes=[
        Route("/", ...),
    ],
    middleware=[
        Middleware(MiddlewareA),
        ),
    ],
)

app = Starlette(
    routes=[
        Route("/", ...),
        Mount("/admin/", admin_app),
    ],
    middleware=[
        Middleware(Middlewareb),
    ],
)
Goldziher commented 2 years ago

Checkout the @asgi decorator

peterschutt commented 2 years ago

Feel free to reopen if that doesn't answer your question @mybigman

mybigman commented 2 years ago

thanks mate... just haven't had a chance to try it yet.

mybigman commented 2 years ago

@Goldziher It appears it doesn't give me what I want or I am going about it the wrong way.

What I was hoping for was

@get("/")
async def hello() -> str:
    return "hello"

test_app = Starlite(route_handlers=[hello])

@asgi(path="/test")
async def testing(scope: Scope, receive: Receive, send: Send) -> None:
    test_app(scope=scope, receive=receive, send=send)

app = Starlite(
    ...
    route_handlers=[testing, routes],
)

Which gives a 500 error ASGI callable returned without starting response.

Can you share an example please.

Goldziher commented 2 years ago

Try to await the test_app

mybigman commented 2 years ago

That fixed the error, however the route isn't available (404) and is not visible in the schema either.

Thoughts?

peterschutt commented 2 years ago
"""
Minimal Starlite application.
"""
from typing import Any

from starlite import Starlite, asgi, get
from starlite.types import Receive, Scope, Send

@get("/")
async def hello_world() -> dict[str, Any]:
    """Hi"""
    return {"hello": "world"}

@asgi("/my-asgi-app")
async def my_asgi_app(scope: Scope, receive: Receive, send: Send) -> None:
    """Mounted ASGI"""
    scope["path"] = "/"
    await Starlite(route_handlers=[hello_world])(scope=scope, receive=receive, send=send)

app = Starlite(route_handlers=[my_asgi_app])
peterschutt commented 2 years ago

however the route isn't available

The reason that the app routing isn't working is due to the path of the scope you are sending through starts with /test in your example. Then you pass that path straight through to the application that only has the path / registered on it.

is not visible in the schema either

Mounting a starlite instance like this doesn't really allow us to inspect the mounted app for it's schema information.. these handlers can literally run any asgi app, not just instances of starlite.

Maybe for the specific case of wanting to mount starlite instances we could allow the to be passed to the Starlite constructor, e.g.,:

a1 = Starlite(...)
a0 = Starlite(route_handlers=[a1, ...])
peterschutt commented 2 years ago

FYI created branch in the hello-world repo with the asgi app example from the docs:

https://github.com/starlite-api/starlite-hello-world/tree/asgi-app-handler

mybigman commented 2 years ago

The reason that the app routing isn't working is due to the path of the scope you are sending through starts with /test in your example. Then you pass that path straight through to the application that only has the path / registered on it.

Sorry this is not clear to me?

Maybe for the specific case of wanting to mount starlite instances we could allow the to be passed to the Starlite constructor, e.g.,:

a1 = Starlite(...)
a0 = Starlite(route_handlers=[a1, ...])

That sounds interesting...

Using what you demonstrated

@get("/")
async def hello() -> str:
    return "hello"

@get("/apple")
async def apple() -> str:
    return "apple"

test_app = Starlite(route_handlers=[hello, apple])

@asgi("/test")
async def testing(scope: Scope, receive: Receive, send: Send) -> None:
    scope["path"] = "/"
    await test_app(scope=scope, receive=receive, send=send)

This works when visiting /test though adding a second route does not /apple or /test/apple

BTW the hello-world branch is not there. I assume it was deleted to use the code above as reference?

peterschutt commented 2 years ago

Sorry this is not clear to me?

The path that you are sending into the mounted starlite application is subject to routing within that second app. The 404 you received before was because you were trying to route path /test in the app that only had a handler registered to /. My example was just a demonstration of that: by changing the path in scope to / it worked because that is the path that the handler was registered to in test_app.

In your most recent example, /test/apple doesn't match any route on app, so the request doesn't have a chance to even reach test_app.

BTW the hello-world branch is not there. I assume it was deleted to use the code above as reference?

It's there for me, weird.

So what are you hoping to achieve by trying to mount another starlite application in the route?

mybigman commented 2 years ago

So what are you hoping to achieve by trying to mount another starlite application in the route?

Basically the same as what fastapi does with sub-application

Its almost like 2 apps in one, each with there own schema, middlewares etc

Make sense?

peterschutt commented 2 years ago

Make sense?

Yes I get that, but what are you trying to achieve with the pattern? Are you solving a specific problem in trying to do this, or just trying to see if it can be done?

mybigman commented 2 years ago

I am not trying to solve a specific problem as such its just how I visualize it when looking at the code, and yes the same can be achieved with a single instance.

I can have the base app instance with its own set of configs/routes/middleware etc and so can the subapp but also inherits from the base app middleware.

Anyway feel free to close.

peterschutt commented 2 years ago

OK, I'll close for now, perhaps others will request the same in the future and we can revisit.

peterschutt commented 2 years ago

Reopened per discussion in discord.

Goldziher commented 2 years ago

Reopened per discussion in discord.

Well, bring the context in 😉

peterschutt commented 2 years ago

Reopened per discussion in discord.

Well, bring the context in wink

I'll give GilFriedEgg a chance to move his discussion over here - but sure, if I'm looking for something to do a bit later and there's been no movement, then I'll spend some time on it.

peterschutt commented 2 years ago

OK so this came up again here

Relevant quote:

Just for cleaner urls. For accounts with vastly different ui based on permissions, I’d rather route to let’s say admin.website.com/control-panel than website.com/admin/control-panel, even if they’re functionally the same.

I asked:

And would they ideally be totally separated Starlite instances?

I commented:

I'd probably use something like this straight away if it was available. E.g., in the example app, the core stuff would be a shared library and the app itself would be configured as a set of micro-services drawing from the shared lib of resources and all pulled together into a master app at the top level

My comment a little misleading - I wouldn't actually change the template app in this manner, but the app I'm working on internally has concerns that are separate enough for this to be introduced there, I think.

peterschutt commented 2 years ago

From discord (GilFriedEgg):

If at all helpful, here are the relevant equivalencies in starlette.

PR: https://github.com/encode/starlette/pull/363

Per Starlette docs, routing to subdomains can be achieved like this:

routes = [
    Host("{subdomain}.example.org", name="sub", app=Router(routes=[
        Mount("/users", name="users", routes=[
            Route("/", user, name="user_list"),
            Route("/{username}", user, name="user_detail")
        ])
    ]))
]
mybigman commented 2 years ago

maybe some traction :)

peterschutt commented 2 years ago

Per Starlette docs, routing to subdomains can be achieved like this:

routes = [
    Host("{subdomain}.example.org", name="sub", app=Router(routes=[
        Mount("/users", name="users", routes=[
            Route("/", user, name="user_list"),
            Route("/{username}", user, name="user_detail")
        ])
    ]))
]

That example isn't really doing anything more than Router already does, right?

I'd say this is the more relevant example from starlette docs:

# This is a standalone static files server:
app = StaticFiles(directory="static")

# This is a static files server mounted within a Starlette application,
# underneath the "/static" path.
routes = [
    ...
    Mount("/static", app=StaticFiles(directory="static"), name="static")
]

app = Starlette(routes=routes)
peterschutt commented 2 years ago

So what should happen when an app is mounted? Should middleware, exception handlers etc that are registered on the main application also apply to a mounted application? Would we just modify the mounted app in place?

E.g., what should happen here:

app_0 = Starlite(..., exception_handlers={500: five_hundred_handler, 400: validation_handler})
app_1 = Starlite(..., exception_handlers={500: five_oh_oh_handler, 404: not_found_handler})
app_0.mount("/app-1", app_1)

What are the exception handlers on app_1 now?

Goldziher commented 2 years ago

Well, starlite is an instance of the statlite router. We can make this work. But I really don't understand why not use routers for this purpose.

Goldziher commented 2 years ago

Ok, so for Host -- the Starlette code looks like this:

    def matches(self, scope: Scope) -> typing.Tuple[Match, Scope]:
        if scope["type"] in ("http", "websocket"):
            headers = Headers(scope=scope)
            host = headers.get("host", "").split(":")[0]
            match = self.host_regex.match(host)
            if match:
                matched_params = match.groupdict()
                for key, value in matched_params.items():
                    matched_params[key] = self.param_convertors[key].convert(value)
                path_params = dict(scope.get("path_params", {}))
                path_params.update(matched_params)
                child_scope = {"path_params": path_params, "endpoint": self.app}
                return Match.FULL, child_scope
        return Match.NONE, {}

Note that hos is a header in this case. So what we are talking about here is actually routing based on the "host" header, rather than handling different domain routing which will be done by a webserver etc.

My position is this--

For "Mount" we use @asgi or ther Router class. I do not understand the point of mounting different starlite apps, and even if there is some edge cases, this is too messy and goes against the main architecture of the framework. If some of you disagree with this, you can convince me otherwise.

For "Host" - we can find a way to handle the header as a path. The only question is the API to use for this. Thoughts and suggestions?

peterschutt commented 2 years ago

Well, starlite is an instance of the statlite router. We can make this work. But I really don't understand why not use routers for this purpose.

From the perspective of separation of concerns, I agree that this adds nothing on top of Router.

What about docs? I wouldn't mind being able to have separate openapi schemas for different routers, so could we look at supporting adding openapi config at the router layer? As you say, Starlite is router, so that should mean we can make the move without having to affect the interface, but then we could support something like this:

v1_router = Router(path="/v1", route_handlers=[...], openapi_config=v1_openapi_config)   # /v1/schema
v2_router = Router(path="/v2", route_handlers=[...], openapi_config=v2_openapi_config)  # /v2/schema
app = Starlite(route_handlers[v1_router, v2_router)

A quick glance at Starlite.create_openapi_schema_model() shows that the only self reference in there is to self.routes which is defined on Router anyway. We could maybe even move that function into a utility in starlite.openapi that accepts a router instance as an argument to produce a schema for any Router or Starlite instance.