uvicorn icon indicating copy to clipboard operation
uvicorn copied to clipboard

support @asynccontextmanager app_factory functions

Open graingert opened this issue 4 years ago • 10 comments

Checklist

  • [x] There are no similar issues or pull requests for this yet.
  • [x] I discussed this idea on the community chat and feedback is positive.

Is your feature related to a problem? Please describe.

Currently the lifespan task runs in a sibling task to any request tasks, but a number of use cases require wrapping a context manager around all the request tasks, eg:

Describe the solution you would like.

support app factories like this:

@contextlib.asynccontextmanager
async def app_factory() -> AsyncGenerator[App, None]:
    async with anyio.create_task_group() as tg:
        app = FastAPI(__name__)
        app.include_router(items.router)
        yield app

or :

var: ContextVar[int] = ContextVar('var', default=42)

@contextlib.asynccontextmanager
async def app_factory():
    token = var.set(999)
    try:
        app = FastAPI(__name__)
        app.include_router(items.router)
        yield app
    finally:
        var.reset(token)

or including the __aenter__/__aexit__ directly on the app instance:

class ACMGRFastAPI(FastAPI):
    async def __aenter__(self) -> "ACMGRFastAPI":
        async with AsyncExitStack() as stack:
            self.database = stack.enter_async_context(database())
            self.cache = stack.enter_async_context(cache())
            self.bot = stack.enter_async_context(bot())
            self._stack = stack.pop_all()

        return self

    async def __aexit__(self, *exc_info) -> bool | None:
        return await self._stack.__aexit__(*exc_info)

Describe alternatives you considered

run lifespan as a parent task to all the request tasks

Additional context

see https://gitter.im/encode/community?at=610989bc8fc359158c4f959e

[!IMPORTANT]

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar

graingert avatar Aug 13 '21 13:08 graingert

Hi @graingert,

Given the amount of changes in #1152, I'm not sure the problem is fully well-posed yet.

For useful background, there's plenty of discussion in #492. I recall we worked hard to minimize the potential for scope creep that arises from the create_app() approach.

The fundamental divide this issue and original discussion in #492 is the architectural decision between FastAPI's Flask-inspired "app factory + blueprints" architecture, and Uvicorn / Starlette's architecture which is typically more globals-intensive. Eg the example in the chat with async with db: yield App(db) is something you can already solve with a resources.py and registering an on_startup / on_shutdown pair (or a lifespan handler).

I'm not sure mixing the two approaches in one project (Uvicorn) is the best approach, or even a safe approach, long term.

Having said that, I have notes more closely related to this proposal.

  • Although we're suggesting an asynccontextmanager solution, the actual content of the context manager for the contextvar + .include_router() does no async/await operation. Is it expected / desirable?
  • In that same example, there is an items value that seems to come from an upper scope (most likely global?). So, it seems that would be confusing the two styles I mentioned above. Is it a sane thing to do and encourage?
  • The ACGMRFastAPI example could also be solved with a simpler await app.on_load() calllback or something, where you'd enter the context managers via the exit stack, and register close callbacks. In a "global-intensive" architecture, it's also possible to set the .database, .cache, etc, variables to None and assign those values in a on_startup callback with a lazy app import. Correct?
  • More generally, I'm not sure about how the examples shown here relate to the "lifespan task vs request task scope" problem statement.

Could we perhaps show a full, complete, real-life scenario, having made sure we went through all possible alternative, more typical arrangements, to make sure there's actually a problem worth solving here?

Hope these questions make sense.

florimondmanca avatar Sep 14 '21 20:09 florimondmanca

I think @graingert can give a much better explanation than I can, but I think the jist of this is that, as a user, you'd expect execution to go something like:

with lifespan():
    app.run()

In other words, running the app happens with in the context of the lifespan.

But in reality it's a whole other beast where the they are executed as sibling tasks. This causes some very unintuitive behavior with contextvars or trio task group cancellation (I can give a concrete example if this is not clear).

For my use, what I'd like to be able to do is set a contexvar value in a lifespan even and get a copy of that context in the endpoint functions.

adriangb avatar Sep 15 '21 00:09 adriangb

florimondmanca (Florimond Manca) the important reason on_startup and on_shutdown don't work is that they're not yield safe, eg this thing: https://discuss.python.org/t/preventing-yield-inside-certain-context-managers/1091

Secondly lifespan is yield safe but it's run at the wrong time in the wrong place, and so a task group throwing a CancelledError into the lifespan task won't have that task bubble up into uvicorn

graingert avatar Sep 15 '21 05:09 graingert

I can give a concrete example if this is not clear

Yes, I think for decision traceability purposes that could be helpful? Perhaps two concrete examples, one for contextvars, one for task group cancellation?

florimondmanca avatar Sep 15 '21 09:09 florimondmanca

the context manager interface allows easy use of task groups like this:

import uvicorn
import anyio


async def throw():
    raise Exception


class App:
    def __init__(self):
        self._task_group = anyio.create_task_group()

    async def __call__(self, scope, recieve, send):
        if scope["type"] == "http":
            await send({
                'type': 'http.response.start',
                'status': 200,
                'headers': [
                    [b'content-type', b'text/plain'],
                ],
            })
            await send({
                'type': 'http.response.body',
                'body': b'Hello, world!',
            })
            self._task_group.start_soon(throw)

    async def __aenter__(self):
        await self._task_group.__aenter__()
        return self

    async def __aexit__(self, *args, **kwargs):
        return await self._task_group.__aexit__(*args, **kwargs)
$ uvicorn app:App --factory
INFO:     Started server process [6504]
INFO:     Waiting for application startup.
INFO:     Application startup complete.
INFO:     Uvicorn running on http://127.0.0.1:8000 (Press CTRL+C to quit)
INFO:     127.0.0.1:58176 - "GET / HTTP/1.1" 200 OK
INFO:     Shutting down
Traceback (most recent call last):
  File "/home/graingert/projects/uvicorn/venv/bin/uvicorn", line 33, in <module>
    sys.exit(load_entry_point('uvicorn', 'console_scripts', 'uvicorn')())
  File "/home/graingert/projects/uvicorn/venv/lib/python3.6/site-packages/click/core.py", line 1137, in __call__
    return self.main(*args, **kwargs)
  File "/home/graingert/projects/uvicorn/venv/lib/python3.6/site-packages/click/core.py", line 1062, in main
    rv = self.invoke(ctx)
  File "/home/graingert/projects/uvicorn/venv/lib/python3.6/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/graingert/projects/uvicorn/venv/lib/python3.6/site-packages/click/core.py", line 763, in invoke
    return __callback(*args, **kwargs)
  File "/home/graingert/projects/uvicorn/uvicorn/main.py", line 425, in main
    run(app, **kwargs)
  File "/home/graingert/projects/uvicorn/uvicorn/main.py", line 447, in run
    server.run()
  File "/home/graingert/projects/uvicorn/uvicorn/server.py", line 74, in run
    return asyncio.get_event_loop().run_until_complete(self.serve(sockets=sockets))
  File "uvloop/loop.pyx", line 1456, in uvloop.loop.Loop.run_until_complete
  File "/home/graingert/projects/uvicorn/uvicorn/server.py", line 78, in serve
    await self.main_loop()
  File "/home/graingert/projects/uvicorn/venv/lib/python3.6/site-packages/contextlib2/__init__.py", line 246, in __aexit__
    await self.gen.athrow(typ, value, traceback)
  File "/home/graingert/projects/uvicorn/uvicorn/server.py", line 108, in serve_acmgr
    logger.info(message, process_id, extra={"color_message": color_message})
  File "/home/graingert/projects/uvicorn/venv/lib/python3.6/site-packages/contextlib2/__init__.py", line 246, in __aexit__
    await self.gen.athrow(typ, value, traceback)
  File "/home/graingert/projects/uvicorn/uvicorn/config.py", line 530, in app_context
    yield
  File "./app.py", line 33, in __aexit__
    return await self._task_group.__aexit__(*args, **kwargs)
  File "/home/graingert/projects/uvicorn/venv/lib/python3.6/site-packages/anyio/_backends/_asyncio.py", line 564, in __aexit__
    raise exceptions[0]
  File "/home/graingert/projects/uvicorn/venv/lib/python3.6/site-packages/anyio/_backends/_asyncio.py", line 601, in _run_wrapped_task
    await coro
  File "./app.py", line 6, in throw
    raise Exception
Exception

graingert avatar Sep 15 '21 10:09 graingert

I think it's nicer with @asynccontextmanager:

import uvicorn
import contextlib2
import anyio


class MyException(Exception):
    pass


async def throw():
    raise MyException


@contextlib2.asynccontextmanager
async def create_app():
    try:
        async with anyio.create_task_group() as tg:
            async def app(scope, recieve, send):
                if scope["type"] == "http":
                    await send({
                        'type': 'http.response.start',
                        'status': 200,
                        'headers': [
                            [b'content-type', b'text/plain'],
                        ],
                    })
                    await send({
                        'type': 'http.response.body',
                        'body': b'Hello, world!',
                    })
                    tg.start_soon(throw)
            yield app
    except MyException:
        print("done!")
$ uvicorn app:create_app --factory
INFO:     Started server process [7224]
INFO:     Waiting for application startup.
INFO:     Application startup complete.
INFO:     Uvicorn running on http://127.0.0.1:8000 (Press CTRL+C to quit)
INFO:     127.0.0.1:58256 - "GET / HTTP/1.1" 200 OK
INFO:     Shutting down
done!

graingert avatar Sep 15 '21 10:09 graingert

I'm not very familiar with contextvars, so here's my attempt at a demo:

import contextvars
import uvicorn
import anyio

g = contextvars.ContextVar('g')

async def throw():
    raise Exception

async def demo():
    g.get()._task_group.start_soon(throw)

class App:
    def __init__(self):
        self._task_group = anyio.create_task_group()

    async def __call__(self, scope, recieve, send):
        if scope["type"] == "http":
            await send({
                'type': 'http.response.start',
                'status': 200,
                'headers': [
                    [b'content-type', b'text/plain'],
                ],
            })
            await send({
                'type': 'http.response.body',
                'body': b'Hello, world!',
            })
            return await demo()

    async def __aenter__(self):
        self._token = g.set(self)
        await self._task_group.__aenter__()
        return self

    async def __aexit__(self, *args, **kwargs):
        v = await self._task_group.__aexit__(*args, **kwargs)
        var.reset(self._token)
        return v

@adriangb has a more full featured usecase over in anydep: https://github.com/adriangb/anydep/issues/1#issuecomment-889936698 see this bit:

Unfortunately it's not that simple. Starlette doesn't process requests in the same context as the lifespan

graingert avatar Sep 15 '21 12:09 graingert

I made a fleshed out example of using anydep for an ASGI app so that this is somewhat realistic: https://github.com/adriangb/anydep/blob/main/comparisons/asgi

The jist of the motivation there is that the dependency injection container has to provide "binds". For requests/response binds, this is done via contextvars (which allows concurrent requests to bind to different Requests objects, database connections, etc.); see here in the example. But for lifespan binding (here in that example) I had to invent a whole concept of "global" scope that modifies the object itself, just because contextvars don't work between lifespans and requests.

If we could have a lifespan-like construct be executed from within the app constructor (App.__aenter__ in @graingert 's example above), then anydep would not need 2 "scope" concepts since it could always just use contextvars to save the current scope state.

adriangb avatar Sep 17 '21 20:09 adriangb

@graingert I'm curious what you think of this: https://github.com/adriangb/lazgi

I'm not sure it's a great idea, but interesting to see that it's feasible!

adriangb avatar Oct 27 '22 04:10 adriangb