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

Import application before running the event loop #1488

Closed
wants to merge 1 commit into from

Conversation

Kludex
Copy link
Member

@Kludex Kludex commented May 15, 2022

@Kludex Kludex requested a review from graingert May 15, 2022 17:10
@Kludex Kludex requested a review from a team May 15, 2022 17:11
@Kludex Kludex requested a review from euri10 June 18, 2022 16:13
@Kludex Kludex mentioned this pull request Jun 18, 2022
5 tasks
@Kludex Kludex added this to the Version 0.18.0 milestone Jun 18, 2022
@graingert
Copy link
Member

how does this interact with reload?

@Kludex
Copy link
Member Author

Kludex commented Jun 19, 2022

@graingert With the simple application:

import anyio


async def example():
    await anyio.sleep(0)
    print("All good!")


anyio.run(example)


async def app(scope, receive, send):
    pass

Running uvicorn with reload:

(uvicorn3.8.12) ➜  uvicorn git:(feat/import-app-before-event-loop) ✗ uvicorn main:app --reload --port 8005
INFO:     Will watch for changes in these directories: ['/Users/marcelotryle/Development/encode/uvicorn']
INFO:     Uvicorn running on http://127.0.0.1:8005 (Press CTRL+C to quit)
INFO:     Started reloader process [6724] using watchgod
All good!
INFO:     Started server process [6738]
INFO:     Waiting for application startup.
INFO:     Application startup complete.
WARNING:  WatchGodReload detected file change in '['/Users/marcelotryle/Development/encode/uvicorn/main.py']'. Reloading...
INFO:     Shutting down
INFO:     Waiting for application shutdown.
INFO:     Application shutdown complete.
INFO:     Finished server process [6738]
All good!
INFO:     Started server process [6774]
INFO:     Waiting for application startup.
INFO:     Application startup complete.

Copy link
Member

@euri10 euri10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm thanks @Kludex

@@ -57,6 +57,8 @@ def __init__(self, config: Config) -> None:

def run(self, sockets: Optional[List[socket.socket]] = None) -> None:
self.config.setup_event_loop()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should call setup_event_loop as close to asyncio.run as possible

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other words... 🤔

@Kludex
Copy link
Member Author

Kludex commented Jun 20, 2022

I'm not able to reproduce the issue that motivated this PR.

I'm going to close this. If someone wants to try to reproduce, and come back with the proof again, we take a decision then.

@Kludex Kludex closed this Jun 20, 2022
@Kludex Kludex deleted the feat/import-app-before-event-loop branch June 20, 2022 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants