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.

Sign up for GitHub

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

Jump to bottom

gh-94597: deprecate asyncio.set_event_loop_policy #110728

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

gh-94597: deprecate asyncio.set_event_loop_policy #110728

wants to merge 15 commits into from
+109 −66

Conversation

graingert
Copy link
Contributor

@graingert graingert commented Oct 11, 2023 •
edited by bedevere-app bot
Loading

@graingert graingert changed the title deprecate asyncio.set_event_loop_policy gh-94597: deprecate asyncio.set_event_loop_policy Oct 11, 2023
@bedevere-app bedevere-app bot mentioned this pull request Oct 11, 2023
Closed
@graingert graingert force-pushed the deprecate-set-event-loop-policy branch 4 times, most recently from a7fe31c to 339445e Compare October 11, 2023 17:18
@graingert graingert force-pushed the deprecate-set-event-loop-policy branch from f7a1b68 to 491b42e Compare October 11, 2023 18:14
@graingert graingert marked this pull request as ready for review October 11, 2023 18:41
cjw296
cjw296 reviewed Oct 11, 2023
View reviewed changes
Copy link
Contributor

@cjw296 cjw296 left a comment

Choose a reason for hiding this comment

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

No objection to the unittest or AsyncMock changes!

@JelleZijlstra JelleZijlstra removed their request for review October 12, 2023 02:18
@rhettinger rhettinger removed their request for review October 16, 2023 03:47
@gvanrossum gvanrossum mentioned this pull request Oct 26, 2023
Merged
gvanrossum
gvanrossum reviewed Oct 26, 2023
View reviewed changes
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Just a few nits, basically LG. That tearDownModule() pattern is annoying! :-)

I assume that deprecating get_event_loop_policy() will follow? Are there other APIs? I'm guessing we also want to avoid instantiating or subclassing the standard event loop policy classes (I expect this will turn up some significant uses of them).

Before we merge this we may need to bring this up on discuss.python.org (in the asyncio category) to see if there are significant objections to the proposed deprecation schedule.

Doc/library/asyncio-policy.rst Outdated Show resolved Hide resolved
Lib/asyncio/events.py Outdated Show resolved Hide resolved
Lib/test/test_asyncio/test_events.py Outdated Show resolved Hide resolved
Misc/NEWS.d/next/Library/2023-10-11-15-55-53.gh-issue-94597.qXo8Q4.rst Outdated Show resolved Hide resolved
@graingert
Copy link
Contributor Author

graingert commented Oct 26, 2023 •
edited
Loading

Just a few nits, basically LG. That tearDownModule() pattern is annoying! :-)

I think we can start getting rid of those tearDownModules using #110774

I assume that deprecating get_event_loop_policy() will follow? Are there other APIs? I'm guessing we also want to avoid instantiating or subclassing the standard event loop policy classes (I expect this will turn up some significant uses of them).

Yep currently that's a bit more complicated because get/set_event_loop calls get_event_loop_policy and afaik we don't want to make get_event_loop an alias of get_running_loop anymore

@hugovk
Copy link
Member

hugovk commented Oct 29, 2023

Please also mention the deprecation in What's New in Python 3.13.

Because it's to be removed in 3.15 or later it could be listed under either Pending Removal in Python 3.15 or Pending Removal in Future Versions.

I suggest the former, as it's more likely to be noticed and action taken, and include the "3.15 or later" wording. We can update later if needed.

hugovk
hugovk reviewed Oct 31, 2023
View reviewed changes
Doc/whatsnew/3.13.rst Outdated Show resolved Hide resolved
@graingert @hugovk
Update Doc/whatsnew/3.13.rst
c81b51a 
Co-authored-by: Hugo van Kemenade <[email protected]>
@graingert
Copy link
Contributor Author

Before we merge this we may need to bring this up on discuss.python.org (in the asyncio category) to see if there are significant objections to the proposed deprecation schedule.

discuss post posted here: https://discuss.python.org/t/removing-the-asyncio-policy-system-asyncio-set-event-loop-policy-in-python-3-15/37553

gvanrossum
gvanrossum approved these changes Oct 31, 2023
View reviewed changes
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Okay, I'm happy with this. Let's wait a bit if anyone brings up substantial objections in the Discourse thread.

willingc
willingc approved these changes Oct 31, 2023
View reviewed changes
Copy link
Contributor

@willingc willingc left a comment

Choose a reason for hiding this comment

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

Bravo @graingert for simplifying. ✨

kumaraditya303
kumaraditya303 requested changes Nov 1, 2023
View reviewed changes
Copy link
Contributor

@kumaraditya303 kumaraditya303 left a comment

Choose a reason for hiding this comment

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

Please also add a notice under asyncio changes, most users like myself won't look under 3.15 removals for this change. Thanks

@bedevere-app
Copy link

bedevere-app bot commented Nov 1, 2023

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be poked with soft cushions!

@graingert
Copy link
Contributor Author

Please also add a notice under asyncio changes, most users like myself won't look under 3.15 removals for this change. Thanks

Can you link me to the location in the docs that you mean please?

@kumaraditya303
Copy link
Contributor

kumaraditya303 commented Nov 1, 2023 •
edited
Loading

Can you link me to the location in the docs that you mean please?

Add asyncio entry in https://docs.python.org/3.13/whatsnew/3.13.html#deprecated

@hugovk
Copy link
Member

hugovk commented Nov 1, 2023

That sounds fine, then let's move it from the "Pending Removal in Python 3.15" section to this one.

@graingert
Copy link
Contributor Author

I have made the requested changes; please review again

gvanrossum
gvanrossum approved these changes Nov 1, 2023
View reviewed changes
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

LGTM, but let's wait until the dust in the Discourse thread settles.

@seifertm seifertm mentioned this pull request Aug 11, 2024
Closed
@1st1
Copy link
Member

1st1 commented Sep 12, 2024

Happy to review and hopefully merge this during the core sprint.

@hugovk hugovk added the sprint label Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hugovk hugovk hugovk left review comments

@cjw296 cjw296 cjw296 left review comments

@kumaraditya303 kumaraditya303 kumaraditya303 requested changes

@gvanrossum gvanrossum gvanrossum approved these changes

@willingc willingc willingc approved these changes

@vsajip vsajip Awaiting requested review from vsajip vsajip is a code owner

@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner

@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner

Assignees
No one assigned
Labels
awaiting merge sprint topic-asyncio
Projects
Sprint 2024
Status: Todo
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@graingert @hugovk @kumaraditya303 @1st1 @cjw296 @willingc @gvanrossum @asvetlov