| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
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
| 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 | ||
| Expand All | @@ -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. | ||
|
|
||
| Expand All | @@ -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 thread
Comment on lines
+38
to
+39
Copy link
Copy Markdown
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. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityIf 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.
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
Author
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. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityI 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
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
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. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityBy 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.
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
Author
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. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityHow about I remove the flag from asyncio.Runner and have callers use a wrapper that mutates the policy loop? @contextmanager
def _with_policy_loop(loop):
try:
asyncio.set_event_loop(loop)
yield
finally:
asyncio.set_event_loop(None)runner = asyncio.Runner()
cmgr = _with_policy_loop(runner.get_loop())
try:
cmgr(runner.run)(corofn())
finally:
cmgr(runner.close)()
I'm proposing this set_policy_loop flag get deprecated at the same time as the policy system. I could add the flag as deprecated in this PR so users won't add it unless they already have more work to do to migrate from the policy system as a concept
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
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. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityI don't think I understand the downside with enabling this by default. My arguments boil down to:
I am not super confident in the event loop policy system, so I won't continue arguing if you don't agree with me after this comment - i'll defer to whenever a core dev takes a look at this issue.
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
Author
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. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
It should be opt in for asyncio.Runner
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
Author
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. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Qualityyep: set_policy_loop defaults to False in asyncio.Runner: def __init__(self, *, debug=None, loop_factory=None, set_policy_loop=False): people using the old asyncio.run get the old behaviour that uses the policy loop factory and sets the policy loop. People using the new asyncio.Runner get the new behaviour which does not use the policy loop factory or set the policy loop by default
Sorry, something went wrong.
All reactions
|
||
|
|
||
| asyncio.run(main(), debug=True) | ||
|
|
||
| Expand All | @@ -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() | ||
| Expand All | @@ -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 | ||
| Expand All | @@ -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 | ||
| Expand All | @@ -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 thread
Comment on lines
+144
to
+145
Copy link
Copy Markdown
Contributor
Author
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. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality@kumaraditya303 it looks like your PR is missing some calls to set_event_loop here
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
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. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Qualityasyncio.Runner should be used a context manager and its close method removes the set event loop so not required here.
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
Author
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. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Qualitythe issue being when Runner.run finishes the loop should be unset with Runner() as runner1, Runner() as runner2:
runner1.run(fn()) # calls set_event_loop
runner2.run(fn()) # also calls set_event_loop
# I'd expect the event loop to be unset here
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
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. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
That happens only if you supply your loop_factory otherwise for existing code, its better to keep it set for existing code not using asyncio.Runner.
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
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. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityI would expect the event loop to be set until the context manager exit which is what we have currently.
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Contributor
Author
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. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityAh I see what you mean
Sorry, something went wrong.
All reactions
|
||
| if (sigint_handler is not None | ||
| and signal.getsignal(signal.SIGINT) is sigint_handler | ||
| ): | ||
| Expand All | @@ -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: | ||
| Expand Down Expand Up | @@ -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 thread
Comment on lines
+187
to
+189
Copy link
Copy Markdown
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. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityThis 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.
Sorry, something went wrong.
All reactions
|
||
| It should be used as a main entry point for asyncio programs, and should | ||
| ideally only be called once. | ||
|
|
||
| Expand All | @@ -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) | ||
|
|
||
|
|
||
| Expand Down | ||
| 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. |
| Back | FazBrowse Home | New Git URL |
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.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Qualitythis opts asyncio.Runner out of the policy system and asyncio.run opts back in
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.