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

gh-126353: remove implicit creation of loop from get_event_loop #126354

Merged
merged 11 commits into from
Nov 4, 2024

Conversation

kumaraditya303
Copy link
Contributor

@kumaraditya303 kumaraditya303 commented Nov 3, 2024

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

Please add also versionchanged directives in the documentation and the What's New entry.

@kumaraditya303 kumaraditya303 added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 3, 2024
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 3, 2024
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

Update also Doc/library/asyncio-policy.rst. Look if there are other mentions of get_event_loop() that needs to be updated.

Doc/library/asyncio-eventloop.rst Outdated Show resolved Hide resolved
Lib/test/test_asyncio/test_events.py Outdated Show resolved Hide resolved
Lib/test/test_asyncio/test_events.py Show resolved Hide resolved
Lib/test/test_asyncio/test_unix_events.py Show resolved Hide resolved
Doc/library/asyncio-eventloop.rst Outdated Show resolved Hide resolved
Lib/test/test_asyncio/test_events.py Outdated Show resolved Hide resolved
Doc/whatsnew/3.14.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@asvetlov asvetlov left a comment

Choose a reason for hiding this comment

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

I was waiting for this date for many years :)

Lib/test/test_asyncio/test_events.py Outdated Show resolved Hide resolved
Lib/test/test_asyncio/test_events.py Outdated Show resolved Hide resolved
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM. 👍

@kumaraditya303 kumaraditya303 merged commit fe5a6ab into python:main Nov 4, 2024
36 checks passed
@kumaraditya303 kumaraditya303 deleted the get-event-loop branch November 4, 2024 08:51
@@ -924,7 +924,7 @@ def test_python_gil(self):
self.assertEqual(proc.stderr, '')

def test_python_asyncio_debug(self):
code = "import asyncio; print(asyncio.get_event_loop().get_debug())"
code = "import asyncio; print(asyncio.new_event_loop().get_debug())"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this needs a with contextlib.closing(... to prevent the test suite printing a ResourceWarning

@@ -576,6 +576,11 @@ asyncio

(Contributed by Kumar Aditya in :gh:`120804`.)

* Removed implicit creation of event loop by :func:`asyncio.get_event_loop`.
It now raises a :exc:`RuntimeError` if there is no current event loop.
(Contributed by Kumar Aditya in :gh:`126353`.)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please mention what should be used instead?

Copy link
Member

Choose a reason for hiding this comment

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

new_event_loop(). But these who need a new even loop already know this.

Copy link
Contributor

Choose a reason for hiding this comment

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

new_event_loop is not correct, it should be asyncio.run or asyncio.Runner

Worst case it would be asyncio.EventLoop, new_event_loop uses the policy system

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants