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

support @asynccontextmanager app_factory functions #1151

Open
2 tasks done
graingert opened this issue Aug 13, 2021 · 9 comments · May be fixed by #1152
Open
2 tasks done

support @asynccontextmanager app_factory functions #1151

graingert opened this issue Aug 13, 2021 · 9 comments · May be fixed by #1152

Comments

@graingert
Copy link
Member

graingert commented Aug 13, 2021

Checklist

  • There are no similar issues or pull requests for this yet.
  • 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
@florimondmanca
Copy link
Member

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.

@adriangb
Copy link
Member

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.

@graingert
Copy link
Member Author

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

@florimondmanca
Copy link
Member

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?

@graingert
Copy link
Member Author

graingert commented Sep 15, 2021

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
Copy link
Member Author

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
Copy link
Member Author

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: adriangb/di#1 (comment)
see this bit:

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

@adriangb
Copy link
Member

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
Copy link
Member

@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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants