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

no longer call set_event_loop in IOLoop.start() #3158

Closed

Conversation

graingert
Copy link
Contributor

@graingert graingert commented Jun 15, 2022

asyncio.set_event_loop was only needed to make asyncio.get_event_loop() work correctly on Python 3.5.3 and earlier, which is no longer supported by tornado

This change is motivated by asyncio.run no longer calling set_event_loop

except (RuntimeError, AssertionError):
old_loop = None # type: ignore
try:
asyncio.set_event_loop(self.asyncio_loop)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

because asyncio.get_event_loop() calls asyncio.get_running_loop() this set_event_loop() is now redundant

@graingert
Copy link
Contributor Author

see also #3156 cc @minrk

@graingert
Copy link
Contributor Author

It looks like the equivalent change in asyncio.run to avoid calling set_event_loop might be causing issues in downstream projects

@graingert graingert marked this pull request as draft June 16, 2022 08:06
@graingert
Copy link
Contributor Author

here's the issue: python/cpython#93896

@bdarnell
Copy link
Member

Yeah, it looks like set_event_loop does more than just determine the result of a future get_event_loop; it is (at least) necessary for the child watcher system so I think it needs to stay until that system is gone. Tornado doesn't directly use child watchers but I think Tornado applications could so we need to stay compatible with it.

@bdarnell bdarnell closed this Jun 17, 2022
@graingert graingert deleted the no-longer-call-set-event-loop branch June 17, 2022 15:32
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.

2 participants