-
-
Notifications
You must be signed in to change notification settings - Fork 745
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
Comments
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 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 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.
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. |
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. |
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 |
Yes, I think for decision traceability purposes that could be helpful? Perhaps two concrete examples, one for |
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 |
I think it's nicer with 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! |
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)
|
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 If we could have a lifespan-like construct be executed from within the app constructor ( |
@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! |
Checklist
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:
or :
or including the
__aenter__/__aexit__
directly on the app instance: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
The text was updated successfully, but these errors were encountered: