Manage asyncio event loops explicitly #998
Merged
Conversation
This file contains 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
The debug mode was needed at the earlier stages of the development but is not needed anymore. However, it brings much inconvenience: The event loop must be known already at the stage of logging configuration, while it is normally created much later in the `kopf.run()`. The `get_event_loop()`, in turn, emits a warning about a deprecated use with no event loop set in advance, which is converted to errors in tests. It seems, the event loop management is supposed to be explicit — so be it. However, trying to `set_event_loop()` up in the stack if CLI commands brings other issues and warnings: there is no place to close that newly created loop unless decorators with context managers are added for all CLI commands. This seems an overkill for the unneeded debug mode. There is the "proper" way to run an implicit loop — `asyncio.run()` — so let's use it. Signed-off-by: Sergey Vasilyev <nolar@nolar.info>
Signed-off-by: Sergey Vasilyev <nolar@nolar.info>
nolar
force-pushed
the
managed-event-loops
branch
from
616bb63
to
e120866
Compare
Signed-off-by: Sergey Vasilyev <nolar@nolar.info>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This should address the failing CI tests with
uvloop, but also to make the event loop more straightforward in general, especially in Python 3.11.1:In Python 3.11.1, a warning was added if
policy.get_event_loop()is called without previously calling thepolicy.set_event_loop(), i.e. when a new loop is implicitly created.Instead, the loops must be created and closed explicitly in the code. Alternatively,
asyncio.run()must be used for managed loops. However, we still have to accept an external loop or default loop for embedded operators — assuming those loops are managed elsewhere.As for the debug mode, it was needed at the earlier stages of the development but is not needed anymore. However, it brings much inconvenience too:
The event loop must be known already at the stage of logging configuration, while it is normally created much later in the
kopf.run(). Theget_event_loop(), in turn, emits a warning about a deprecated use with no event loop set in advance, which is converted to errors in tests.Trying to fix it leads to overly complicated solutions, which look like overkill for the unneeded debug mode. It is easier to remove it.