-
-
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
Eager Asynchronous Construction of App Interferes with Running Event Loop #941
Comments
@MarioIshac Hello, To be honest, this is looking like a very odd setup to me. Have you considered leveraging the ASGI lifespan functionality? Apps can receive "startup" and "shutdown" events. If startup event handlers fail then Uvicorn will fast exit, just as the use case you described. I believe this would be a much saner approach than a "create app a synchronously". Other than that, to resolve the "bug" you're seeing, you'd have to properly close the event loop once your app has been created. Right now you're leaving it hanging unclosed, but Uvicorn expects a clean slate async environment. |
@MarioIshac Ill close this preemptively since I think you should be able to figure things out given my hints above. Don't hesitate to reach back if need be. Thanks! |
Hi @florimondmanca, Thank you for your reply. I'll admit I only recently jumped into I actually do use ASGI lifespan functionality to handle initialization that needs to be done per-process (because of library incompatibility with gunicorns
On the branch I made, running with So while the ASGI startup event functionality does come close (it is still eager), I haven't found a way to make it benefit from preloading. Separate from this, I have a question about the last point you made. Since the app is loaded from a string, I thought uvicorn would be still creating the event loop (and selecting
to this:
So |
@MarioIshac When you run config = Config(...)
server = Server(config)
server.run() The event loop is set up by def appmodule(): # your code
loop = asyncio.get_event_loop()
return loop.run_until_complete(await getapp()) # XXX: Can't do this: `loop` is already running!
async def serve(): # uvicorn code
app = appmodule().app
await _mainloop(app)
# uvicorn code
loop = asyncio.new_event_loop()
loop.run_until_complete(serve())
loop.close() |
@florimondmanca Yup I looked over this section of
What is the downside of such an approach? As far as I understand |
@MarioIshac Okay — my bad, I didn't actually see your PR #942. :-) That's interesting, let me take a look. |
|
I think this would be fixed by #1151 |
I have a similar issue, when trying to use the How should I fix this? Do I need two different loops to happen on parent process level or each forked worker needs a fresh own loop ? |
https://bugs.python.org/issue21998 asyncio can't survive a fork, you have to start a new loop and make sure the old loop has finished before forking - or use spawn |
+1 |
1 similar comment
+1 |
Because
Server#run
callsConfig#load
withinloop.run_until_complete(self.serve(...))
, a config load that involves asynchronously constructing an app fails because the event loop is already running. Note that this config load will fail only if the app name is provided instead of the app itself. This is because in the latter case, the app had already been asynchronously constructed in its ownrun_until_complete
prior to therun_until_complete
launched byServer
.To reproduce
pip install -e git+https://github.com/encode/uvicorn#egg=uvicorn
main.py
:uvicorn main:app
.Expected behavior
And that a server request would return Hello, world!
Actual behavior
Event loop started running before app could be asynchronously constructed.
Environment
Ubuntu 20.04
CPython 3.8.5
Uvicorn 0.13.3
Additional context
Replacing step 1 with
pip install -e git+https://github.com/MarioIshac/uvicorn@feat-async-construction#egg=uvicorn
will achieve the expected behavior.The particular benefit I get from having the app asynchronously & eagerly constructed instead of lazily constructed within
async def app(...)
is that I can take advantage of gunicorn preloading. I can also have the server fail-fast instead of on-first-request in the case that the asynchronous construction fails (for example, database connection pool could not be created).Important
The text was updated successfully, but these errors were encountered: