Skip to content

bpo-39529: Deprecate creating new event loop in asyncio.get_event_loop() - #23554

Merged
serhiy-storchaka merged 12 commits into
python:masterfrom
serhiy-storchaka:asyncio-get_event_loop
Apr 25, 2021
Merged

bpo-39529: Deprecate creating new event loop in asyncio.get_event_loop()#23554
serhiy-storchaka merged 12 commits into
python:masterfrom
serhiy-storchaka:asyncio-get_event_loop

Conversation

@serhiy-storchaka

@serhiy-storchakaserhiy-storchaka commented Nov 29, 2020

Copy link
Copy Markdown
Member

@serhiy-storchaka

Copy link
Copy Markdown
MemberAuthor

It is a draft. Tests which use asyncio.gather(), asyncio.as_completed(), etc in synchronous code should be rewritten to call them in coroutines, and purposed tests for warnings and errors should be added.

@asvetlovasvetlov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good so far.

Comment threadLib/asyncio/tasks.py Outdated
@serhiy-storchaka
serhiy-storchaka marked this pull request as ready for review December 6, 2020 18:26
@serhiy-storchaka

Copy link
Copy Markdown
MemberAuthor

Ready for review.

Comment threadLib/asyncio/tasks.py
loop = events.get_event_loop()
task = loop.create_task(coro_or_future)
if task._source_traceback:
del task._source_traceback[-1]

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

It was removed because this code did not work as intented long time ago (if worked at all).

res = loop.run_until_complete(asyncio.wait_for(coro(), timeout=None))
self.assertEqual(res, 'done')

def test_wait_for_with_global_loop(self):

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

It is virtually a duplicate of the above test. Initially they were different by passing the loop argument or not, now it is removed

self.assertAlmostEqual(0.15, loop.time())
self.assertEqual(res, 42)

def test_wait_with_global_loop(self):

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

It is virtually a duplicate of the above test. Initially they were different by passing the loop argument or not, now it is removed

self.assertEqual(y, 'b')
self.assertAlmostEqual(0.10, loop.time())

x = loop.run_until_complete(futs[1])

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Currently two parts of the test was virtually duplicates. Added new tests (test_as_completed_coroutine_without_loop and below) which are essentially different.

@serhiy-storchaka

Copy link
Copy Markdown
MemberAuthor

Please give attention to tests. This is the main part of the PR.

@rhettinger
rhettinger removed their request for review December 19, 2020 04:30
@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actionsgithub-actionsBot added the stale Stale PR or inactive for long period of time. label Jan 19, 2021

@asvetlovasvetlov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, please go ahead.
Sorry for the very long review.

@serhiy-storchaka
serhiy-storchaka merged commit 172c0f2 into python:masterApr 25, 2021
@serhiy-storchaka
serhiy-storchaka deleted the asyncio-get_event_loop branch April 25, 2021 10:40
@serhiy-storchakaserhiy-storchaka removed the stale Stale PR or inactive for long period of time. label Aug 31, 2021
@asvetlovasvetlov mentioned this pull request Jun 1, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-featureA feature request or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@serhiy-storchaka@asvetlov@the-knights-who-say-ni@bedevere-bot