-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
ref: Remove usage of deprecated get_event_loop #4697
base: main
Are you sure you want to change the base?
Conversation
CodSpeed Performance ReportMerging #4697 will improve performances by 56.75%Comparing Summary
Benchmarks breakdown
|
if loop.is_running(): | ||
# Run the coroutine in a separate event loop in a new thread | ||
return run_in_thread(coro) | ||
return loop.run_until_complete(coro) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the loop is not running, this will most probably not work.
|
||
|
||
def run_until_complete(coro): | ||
try: | ||
loop = asyncio.get_event_loop() | ||
if loop.is_running(): | ||
# Run the coroutine in a separate event loop in a new thread |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the advantage of running in a separate thread+loop ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't remember. Maybe related to the this event loop is already running
error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. We should probably add tests.
Remove usage of deprecated get_event_loop