Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Replace use of the deprecated asyncio.get_event_loop function. #492

Merged
merged 11 commits into from
Jan 6, 2022

Conversation

mdickinson
Copy link
Member

@mdickinson mdickinson commented Dec 16, 2021

This PR replaces the use of asyncio.get_event_loop in the Traits Futures core. There's a remaining example that still uses get_event_loop, but I'll address that separately.

There are some remaining warnings in the test run resulting from the fact that we have no way to clean up asyncio event loops created through ETSToolkit, but I believe ETSToolkit should not have been supporting asyncio in the first place - see #490.

Together with #490, this addresses #491.

Also closes #333.

@@ -9,7 +9,7 @@
# Thanks for using Enthought open source!

"""
IEventLoop implementation for the main-thread asyncio event loop.
IEventLoop implementation wrapping an asyncio event loop.
Copy link
Member Author

Choose a reason for hiding this comment

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

Docstring changed because with Python's migration away from get_event_loop, "the main-thread asyncio event loop" doesn't really make sense any more.

@mdickinson
Copy link
Member Author

There's a remaining example that still uses get_event_loop, but I'll address that separately.

On second thoughts, this PR leaves that example actually broken: it simply hangs when run, so that should be fixed as part of this PR. This may require a bit of reworking.

@mdickinson mdickinson marked this pull request as draft December 16, 2021 17:48
traits_future = submit_iteration(traits_executor, approximate_pi)
traits_future.observe(print_progress, "result_event")

# For Python 3.7 and later, just use asyncio.run.
Copy link
Member Author

Choose a reason for hiding this comment

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

Comment removed: asyncio.run isn't the right thing to use here, since it creates its own new event loop. We don't want that - we want to use the event loop that Traits Futures is delivering results to.

@mdickinson
Copy link
Member Author

that should be fixed as part of this PR. This may require a bit of reworking.

Done. Ready for review.

@mdickinson mdickinson marked this pull request as ready for review December 16, 2021 18:17
@rahulporuri rahulporuri self-requested a review December 17, 2021 08:36
traits_futures/asyncio/event_loop.py Outdated Show resolved Hide resolved
@mdickinson
Copy link
Member Author

CI failure was due to out-of-date copyright years; should be fixed in #494.

@mdickinson mdickinson merged commit 4530c9d into main Jan 6, 2022
@mdickinson mdickinson deleted the fix/get-event-loop branch January 6, 2022 08:56
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow event_loop to be specified when creating AsyncioContext
2 participants