gh-94597: add asyncio.EventLoop by graingert · Pull Request #110723 · python/cpython (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation13 Commits9 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Contributor
•
edited by github-actionsbot
Loading
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()
GH-94597 add asyncio.EventLoop gh-94597: add asyncio.EventLoop
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?
Comment on lines 1728 to 1729
| Uses the most efficient available event loop for the given |
|---|
| platform. |
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.
…PC8t.rst
Co-authored-by: Guido van Rossum gvanrossum@gmail.com
Comment on lines 1728 to 1729
| Uses the most efficient available event loop for the given |
|---|
| platform. |
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?
| 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. |
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).
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
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
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Merging now.
It is an alias to :class:
SelectorEventLoopon Unix and :class:ProactorEventLoopon 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).
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request
This is needed to pave the way for deprecating and eventually killing the event loop policy system (which is over-engineered and rarely used).
Reviewers
