GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-94597: add asyncio.EventLoop by graingert · Pull Request #110723 · python/cpython

Conversation

@graingert

@graingert graingert commented Oct 11, 2023 •
edited by github-actions Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor
  • Issue: deprecate the asyncio child watchers system #94597

📚 Documentation preview 📚: https://cpython-previews--110723.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@graingert
pythonGH-94597 add asyncio.EventLoop
d2f7090
@bedevere-app bedevere-app Bot mentioned this pull request Oct 11, 2023
deprecate the asyncio child watchers system #94597
Closed
@blurb-it
📜🤖 Added by blurb_it.
47ea029
graingert
graingert commented Oct 11, 2023
View reviewed changes
Comment thread Doc/library/asyncio-eventloop.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@graingert
Update Doc/library/asyncio-eventloop.rst
dc9cfee
@graingert

graingert commented Oct 11, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

This allows running asyncio coroutines without interacting with the policy system eg:

asyncio.run(main(), loop_factory=asyncio.EventLoop)
# no need to call asyncio.set_event_loop_policy(None) !

this allows code to run without interfering with a policy that has been set in one location especially one that expects asyncio.set_event_loop() to be called after every invocation of asyncio.new_event_loop()
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
graingert added 2 commits October 11, 2023 16:14
@graingert
add EventLoop to relevant __all__s
2beab6e
@graingert
add missing sys import
6290284
@graingert graingert changed the title GH-94597 add asyncio.EventLoop gh-94597: add asyncio.EventLoop Oct 11, 2023
@graingert
graingert marked this pull request as ready for review October 11, 2023 16:07
@graingert
graingert requested review from 1st1, asvetlov, gvanrossum, kumaraditya303 and willingc as code owners October 11, 2023 16:07
@bedevere-app bedevere-app Bot added the awaiting review label Oct 11, 2023
@graingert
Merge branch 'main' into add-asyncio-event-loop
ac27e15
gvanrossum
gvanrossum reviewed Oct 12, 2023
View reviewed changes

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

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.

Do you intend this to be merged before or after the deprecations PR?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/library/asyncio-eventloop.rst Outdated
Comment on lines +1728 to +1729
Uses the most efficient available event loop for the given
platform.

@gvanrossum gvanrossum Oct 12, 2023

Copy link
Copy Markdown
Member

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 think this ought to explain that it's an alias for one of the above classes, and that it's the one used by default, and that it's selected the way described above (L1689-90). "Uses" doesn't even explain that it's a subclass of AbstractEventLoop.

Maybe we should be less specific about how the default is selected, and probably whatever we say about it should be only said once, but it should be made clear that this class is the default, and the default is this class.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Misc/NEWS.d/next/Library/2023-10-11-15-07-21.gh-issue-94597.NbPC8t.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@graingert @gvanrossum
Update Misc/NEWS.d/next/Library/2023-10-11-15-07-21.gh-issue-94597.Nb…
ad2c22c 
…PC8t.rst

Co-authored-by: Guido van Rossum <[email protected]>
graingert
graingert commented Oct 12, 2023
View reviewed changes
Comment thread Doc/library/asyncio-eventloop.rst Outdated
Comment on lines +1728 to +1729
Uses the most efficient available event loop for the given
platform.

@graingert graingert Oct 12, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

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.

maybe something like this? The other classes don't explain that they are subclasses of AbstractEventLoop, should that be explained in ProactorEventLoop and SelectorEventLoop?

Suggested change
Uses the most efficient available event loop for the given
platform.
Uses the most efficient available event loop for the given
platform.
It is an alias for :class:`SelectorEventLoop` on Unix and :class:`ProactorEventLoop` on Windows.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@gvanrossum gvanrossum Oct 12, 2023

Copy link
Copy Markdown
Member

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.

Not quite. I don't like any sentence describing a class object starting with "Uses". Classes are nouns, and the description should explain what the class is, not what it does. Also, the current algorithm for choosing a default is now specified in two places. I think it should be specified only here (and the earlier text should be changed to claim that EventLoop is the class used by default).

It's also a good idea to clarify that both concrete classes are subclasses of AbstractEventLoop (but if you're tired of going around in review you can skip that).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@graingert graingert Oct 12, 2023

Copy link
Copy Markdown
Contributor Author

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.

ok I've pushed a change with some doc tweaks

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@graingert

graingert commented Oct 12, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

Do you intend this to be merged before or after the deprecations PR?

I don't mind what order the PRs go in - I plan a cleanup PR for the CPython test suite removing redundant calls to set_event_loop_policy(None) once asyncio.EventLoop is available for use in asyncio.run calls
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@graingert
asyncio.EventLoop docs clarification
27d96ca
vstinner
vstinner reviewed Oct 12, 2023
View reviewed changes
Comment thread Doc/library/asyncio-eventloop.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
graingert
graingert commented Oct 12, 2023
View reviewed changes
Comment thread Doc/library/asyncio-eventloop.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@graingert
remove .. availability tag from asyncio.EventLoop
afaecf2 
it's always available
@kumaraditya303 kumaraditya303 self-assigned this Oct 12, 2023
gvanrossum
gvanrossum approved these changes Oct 12, 2023
View reviewed changes

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

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.

Thanks! Merging now.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting core review and removed awaiting review labels Oct 12, 2023
gvanrossum
gvanrossum approved these changes Oct 12, 2023
View reviewed changes
@gvanrossum
gvanrossum merged commit 8c6c14b into python:main Oct 12, 2023
@bedevere-app bedevere-app Bot removed the awaiting core review label Oct 12, 2023
@graingert
graingert deleted the add-asyncio-event-loop branch October 12, 2023 14:14
@vstinner

vstinner commented Oct 12, 2023

Copy link
Copy Markdown
Member

It is an alias to :class:SelectorEventLoop on Unix and :class:ProactorEventLoop on Windows.

Well. In the early days of tulip/asyncio, there were more choices, and we didn't know how third-party projects will inherit/override these classes. Now I agree that asyncio.EventLoop is the way to go, it's easier to remind, it's more explicit, and it's easy to guess what is it (SelectorEventLoop or ProactorEventLoop).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@graingert @Glyphack
pythongh-94597: Add asyncio.EventLoop (python#110723)
356e4bb 
This is needed to pave the way for deprecating and eventually killing the event loop policy system (which is over-engineered and rarely used).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner left review comments
@gvanrossum gvanrossum gvanrossum approved these changes
@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner
@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner
@kumaraditya303 kumaraditya303 Awaiting requested review from kumaraditya303 kumaraditya303 is a code owner
@willingc willingc Awaiting requested review from willingc willingc is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@kumaraditya303 kumaraditya303

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@graingert @vstinner @gvanrossum @kumaraditya303
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.