Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 35.2k
gh-93896: allow enabling asyncio.Runner calls to asyncio.set_event_loop independantly to loop_factory#94058
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
f6b7bab428cc8a7786b44abbd6b4d11310235e746f489d5449f0f5191431a6345b1af028af30883b15145865ac6File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| __all__ = ('Runner', 'run') | ||
| import sys | ||
| import contextvars | ||
| import enum | ||
| import functools | ||
| @@ -17,6 +18,12 @@ class _State(enum.Enum): | ||
| CLOSED = "closed" | ||
| if sys.platform == "win32": | ||
| from .windows_events import ProactorEventLoop as _new_event_loop | ||
| else: | ||
| from .unix_events import SelectorEventLoop as _new_event_loop | ||
| class Runner: | ||
| """A context manager that controls event loop life cycle. | ||
| @@ -25,7 +32,11 @@ class Runner: | ||
| and properly finalizes the loop at the context manager exit. | ||
| If debug is True, the event loop will be run in debug mode. | ||
| If loop_factory is passed, it is used for new event loop creation. | ||
| If loop_factory is passed, it is used for new event loop creation, otherwise | ||
| a ProactorEventLoop will be started on Windows or a SelectorEventLoop will | ||
| be started on unix | ||
| If set_policy_loop is True, the event loop in the default policy will be | ||
| set. | ||
Comment on lines
+38
to
+39
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this is classified as a bug (clearly it is causing issues), then fixing it shouldn't be blocking. By only implementing this behind an argument we are introducing a new feature which means that this will only be added to 3.12 (and not 3.11). I would vote for not adding this argument. ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's important to maintain the asyncio.Runner support for a high level policy-free use of asyncio, so that the policy system can be deprecated and removed Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By adding this option we make it more difficult to deprecate the policy system because then this option will need to be deprecated as well. Users who specify this option will have more to do to migrate.
| ||
| asyncio.run(main(), debug=True) | ||
| @@ -45,14 +56,18 @@ class Runner: | ||
| # Note: the class is final, it is not intended for inheritance. | ||
| def __init__(self, *, debug=None, loop_factory=None): | ||
| def __init__(self, *, debug=None, loop_factory=None, set_policy_loop=None): | ||
| self._state = _State.CREATED | ||
| self._debug = debug | ||
| self._loop_factory = loop_factory | ||
| self._loop = None | ||
| self._context = None | ||
| self._interrupt_count = 0 | ||
| self._set_event_loop = False | ||
| if set_policy_loop is None and loop_factory is None: | ||
| # default to setting the policy loop, if the loop_factory is not set | ||
| self._set_policy_loop = True | ||
| else: | ||
| self._set_policy_loop = set_policy_loop | ||
| def __enter__(self): | ||
| self._lazy_init() | ||
| @@ -67,11 +82,13 @@ def close(self): | ||
| return | ||
| try: | ||
| loop = self._loop | ||
| if self._set_policy_loop: | ||
| events.set_event_loop(loop) | ||
| _cancel_all_tasks(loop) | ||
| loop.run_until_complete(loop.shutdown_asyncgens()) | ||
| loop.run_until_complete(loop.shutdown_default_executor()) | ||
| finally: | ||
| if self._set_event_loop: | ||
| if self._set_policy_loop: | ||
| events.set_event_loop(None) | ||
| loop.close() | ||
| self._loop = None | ||
| @@ -93,10 +110,11 @@ def run(self, coro, *, context=None): | ||
| "Runner.run() cannot be called from a running event loop") | ||
| self._lazy_init() | ||
| loop = self._loop | ||
| if context is None: | ||
| context = self._context | ||
| task = self._loop.create_task(coro, context=context) | ||
| task = loop.create_task(coro, context=context) | ||
| if (threading.current_thread() is threading.main_thread() | ||
| and signal.getsignal(signal.SIGINT) is signal.default_int_handler | ||
| @@ -114,15 +132,17 @@ def run(self, coro, *, context=None): | ||
| self._interrupt_count = 0 | ||
| try: | ||
| if self._set_event_loop: | ||
| events.set_event_loop(self._loop) | ||
| return self._loop.run_until_complete(task) | ||
| if self._set_policy_loop: | ||
| events.set_event_loop(loop) | ||
| return loop.run_until_complete(task) | ||
| except exceptions.CancelledError: | ||
| if self._interrupt_count > 0 and task.uncancel() == 0: | ||
| raise KeyboardInterrupt() | ||
| else: | ||
| raise # CancelledError | ||
| finally: | ||
| if self._set_policy_loop: | ||
| events.set_event_loop(None) | ||
Comment on lines
+144
to
+145
ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kumaraditya303 it looks like your PR is missing some calls to set_event_loop here Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
| ||
| if (sigint_handler is not None | ||
| and signal.getsignal(signal.SIGINT) is sigint_handler | ||
| ): | ||
| @@ -134,8 +154,7 @@ def _lazy_init(self): | ||
| if self._state is _State.INITIALIZED: | ||
| return | ||
| if self._loop_factory is None: | ||
| self._loop = events.new_event_loop() | ||
| self._set_event_loop = True | ||
| self._loop = _new_event_loop() | ||
| else: | ||
| self._loop = self._loop_factory() | ||
| if self._debug is not None: | ||
| @@ -165,7 +184,9 @@ def run(main, *, debug=None): | ||
| If debug is True, the event loop will be run in debug mode. | ||
| This function always creates a new event loop and closes it at the end. | ||
| This function always creates a new event loop from the default policy sets | ||
| it in the event loop policy and closes it at the end and sets the policy | ||
| loop to None. | ||
Comment on lines
+187
to
+189
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This long sentence is quite wordy and without knowing the change I would probably not understand what it means. I would probably suggest not making this change (referring to this very sentence). To most, setting the loop in the event loop policy is probably synonymous to actually creating and running a loop. | ||
| It should be used as a main entry point for asyncio programs, and should | ||
| ideally only be called once. | ||
| @@ -182,7 +203,11 @@ async def main(): | ||
| raise RuntimeError( | ||
| "asyncio.run() cannot be called from a running event loop") | ||
| with Runner(debug=debug) as runner: | ||
| with Runner( | ||
| debug=debug, | ||
| loop_factory=events.new_event_loop, | ||
| set_policy_loop=True, | ||
| ) as runner: | ||
| return runner.run(main) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Restored missing ``set_event_loop()`` calls in ``asyncio.run()`` and ``AsyncTestCase`` accidentally removed in the 3.11 betas. |
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.
this opts asyncio.Runner out of the policy system and asyncio.run opts back in