From 5103821415747bd4f88eff817453cf8b777bd976 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 2 Jun 2023 14:12:15 +0200 Subject: [PATCH 01/20] Add __str__ and __repr__ to Timer classes This is to make it easier to debug and to quickly print a timer. Signed-off-by: Leandro Lucarella --- src/frequenz/channels/util/_timer.py | 43 ++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/src/frequenz/channels/util/_timer.py b/src/frequenz/channels/util/_timer.py index d49a3664..1145d9e6 100644 --- a/src/frequenz/channels/util/_timer.py +++ b/src/frequenz/channels/util/_timer.py @@ -66,6 +66,14 @@ def calculate_next_tick_time( """ return 0 # dummy value to avoid darglint warnings + def __repr__(self) -> str: + """Return a string representation of the instance. + + Returns: + The string representation of the instance. + """ + return f"{type(self).__name__}()" + class TriggerAllMissed(MissedTickPolicy): """A policy that triggers all the missed ticks immediately until it catches up. @@ -242,6 +250,22 @@ def calculate_next_tick_time( return now + interval return scheduled_tick_time + interval + def __str__(self) -> str: + """Return a string representation of the instance. + + Returns: + The string representation of the instance. + """ + return f"{type(self).__name__}({self.delay_tolerance})" + + def __repr__(self) -> str: + """Return a string representation of the instance. + + Returns: + The string representation of the instance. + """ + return f"{type(self).__name__}({self.delay_tolerance=})" + class Timer(Receiver[timedelta]): """A timer receiver that triggers every `interval` time. @@ -681,3 +705,22 @@ def _now(self) -> int: The current monotonic clock time in microseconds. """ return _to_microseconds(self._loop.time()) + + def __str__(self) -> str: + """Return a string representation of the timer. + + Returns: + The string representation of the timer. + """ + return f"{type(self).__name__}({self.interval})" + + def __repr__(self) -> str: + """Return a string representation of the timer. + + Returns: + The string representation of the timer. + """ + return ( + f"{type(self).__name__}<{self.interval=}, {self.missed_tick_policy=}, " + f"{self.loop=}, {self.is_running=}>" + ) From 2c173d32ad858409b8995a814d5409259ab92cc8 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Thu, 8 Jun 2023 15:23:14 +0200 Subject: [PATCH 02/20] Make type hints use built-in types Signed-off-by: Leandro Lucarella --- src/frequenz/channels/util/_select.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/frequenz/channels/util/_select.py b/src/frequenz/channels/util/_select.py index 66868122..949a9347 100644 --- a/src/frequenz/channels/util/_select.py +++ b/src/frequenz/channels/util/_select.py @@ -11,7 +11,7 @@ import asyncio import logging from dataclasses import dataclass -from typing import Any, Dict, List, Optional, Set, TypeVar +from typing import Any, TypeVar from .._base_classes import Receiver from .._exceptions import ReceiverStoppedError @@ -28,7 +28,7 @@ class _Selected: receiver gets closed. """ - inner: Optional[Any] + inner: Any @dataclass @@ -41,7 +41,7 @@ class _ReadyReceiver: When a channel has closed, `recv` should be `None`. """ - recv: Optional[Receiver[Any]] + recv: Receiver[Any] | None def get(self) -> _Selected: """Consume a message from the receiver and return a `_Selected` object. @@ -101,14 +101,14 @@ def __init__(self, **kwargs: Receiver[Any]) -> None: **kwargs: sequence of receivers """ self._receivers = kwargs - self._pending: Set[asyncio.Task[bool]] = set() + self._pending: set[asyncio.Task[bool]] = set() for name, recv in self._receivers.items(): self._pending.add(asyncio.create_task(recv.ready(), name=name)) self._ready_count = 0 self._prev_ready_count = 0 - self._result: Dict[str, Optional[_ReadyReceiver]] = { + self._result: dict[str, _ReadyReceiver | None] = { name: None for name in self._receivers } @@ -138,7 +138,7 @@ async def ready(self) -> bool: # pylint: disable=too-many-nested-blocks if self._ready_count > 0: if self._ready_count == self._prev_ready_count: - dropped_names: List[str] = [] + dropped_names: list[str] = [] for name, value in self._result.items(): if value is not None: dropped_names.append(name) @@ -185,7 +185,7 @@ async def ready(self) -> bool: self._pending.add(asyncio.create_task(recv.ready(), name=name)) return True - def __getattr__(self, name: str) -> Optional[Any]: + def __getattr__(self, name: str) -> Any: """Return the latest unread message from a `Receiver`, if available. Args: From 5166b27b3a18f510da24e912e8c99fe79d6009da Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Tue, 13 Jun 2023 14:14:43 +0200 Subject: [PATCH 03/20] Remove unused __future__.annotations Signed-off-by: Leandro Lucarella --- tests/utils/test_integration.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/utils/test_integration.py b/tests/utils/test_integration.py index 25700784..2976f1d4 100644 --- a/tests/utils/test_integration.py +++ b/tests/utils/test_integration.py @@ -3,8 +3,6 @@ """Integration tests for the `util` module.""" -from __future__ import annotations - import os import pathlib from datetime import timedelta From 71306e75c320ad0bb021396b82ebbd5338a1fc38 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Tue, 13 Jun 2023 14:14:57 +0200 Subject: [PATCH 04/20] Remove type hints from docstrings Signed-off-by: Leandro Lucarella --- tests/utils/test_integration.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/utils/test_integration.py b/tests/utils/test_integration.py index 2976f1d4..9aa6de41 100644 --- a/tests/utils/test_integration.py +++ b/tests/utils/test_integration.py @@ -17,8 +17,7 @@ async def test_file_watcher(tmp_path: pathlib.Path) -> None: """Ensure file watcher is returning paths on file events. Args: - tmp_path (pathlib.Path): A tmp directory to run the file watcher on. - Created by pytest. + tmp_path: A tmp directory to run the file watcher on. Created by pytest. """ filename = tmp_path / "test-file" file_watcher = FileWatcher(paths=[str(tmp_path)]) @@ -56,8 +55,7 @@ async def test_file_watcher_deletes(tmp_path: pathlib.Path) -> None: the file doesn't exist. Args: - tmp_path (pathlib.Path): A tmp directory to run the file watcher on. - Created by pytest. + tmp_path: A tmp directory to run the file watcher on. Created by pytest. """ filename = tmp_path / "test-file" file_watcher = FileWatcher( From 2764d0171d926bc2e41c29eca45a719fcc797f3f Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 9 Jun 2023 12:21:50 +0200 Subject: [PATCH 05/20] Add a new async iterable select() function This function will replace the current `Select` implementation with the following improvements: * Proper type hinting by using the new helper type guard `selected_from()`. * Fixes potential starvation issues. * Simplifies the interface by providing values one-by-one. * Simplifies the implementation, so it is easier to maintain. There are some other improvements we would have liked to be able to make but was difficult. For example, typing for `select()` is tricky. We had the idea of using a declarative design, something like: ```python class MySelector(Selector): receiver1: x.new_receiver() receiver2: y.new_receiver() async for selected in MySelector: if selected.receiver is receiver1: # Do something with selected.value elif selected.receiver is receiver1: # Do something with selected.value ``` This is similar to `Enum`, but `Enum` has special support in `mypy` that we can't have. With the current implementation, the typing could be slightly improved by using `TypeVarTuple`, but we are not because "transformations" are not supported yet, see: https://github.com/python/typing/issues/1216 Also support for `TypeVarTuple` in general is still experimental (and very incomplete in `mypy`). With this we would also probably be able to properly type `select` and *maybe* even be able to leverage the exhaustiveness checking of `mypy` to make sure the selected value is narrowed down to the correct type to make sure all receivers are handled, with the help of `assert_never` as described in: https://docs.python.org/3.11/library/typing.html#typing.assert_never We also explored the possibility of using `match` to perform exhaustiveness checking, but we couldn't find a way to make it work with `match`, and `match` is not yet checked for exhaustiveness by `mypy` anyway, see: https://github.com/python/mypy/issues/13597 Signed-off-by: Leandro Lucarella --- src/frequenz/channels/util/__init__.py | 21 +- src/frequenz/channels/util/_select.py | 395 ++++++++++++++++++++++++- 2 files changed, 412 insertions(+), 4 deletions(-) diff --git a/src/frequenz/channels/util/__init__.py b/src/frequenz/channels/util/__init__.py index 72369a7b..d9f5a29f 100644 --- a/src/frequenz/channels/util/__init__.py +++ b/src/frequenz/channels/util/__init__.py @@ -23,12 +23,23 @@ * [Select][frequenz.channels.util.Select]: A helper to select the next available message for each [receiver][frequenz.channels.Receiver] in a group of receivers. + +* [select][frequenz.channels.util.select]: A function to iterate over a group + of [receivers][frequenz.channels.Receiver] and select the next available value. """ from ._file_watcher import FileWatcher from ._merge import Merge from ._merge_named import MergeNamed -from ._select import Select +from ._select import ( + Select, + Selected, + SelectError, + SelectErrorGroup, + UnhandledSelectedError, + select, + selected_from, +) from ._timer import ( MissedTickPolicy, SkipMissedAndDrift, @@ -42,9 +53,15 @@ "Merge", "MergeNamed", "MissedTickPolicy", - "Timer", "Select", + "SelectError", + "SelectErrorGroup", + "Selected", "SkipMissedAndDrift", "SkipMissedAndResync", + "Timer", "TriggerAllMissed", + "UnhandledSelectedError", + "select", + "selected_from", ] diff --git a/src/frequenz/channels/util/_select.py b/src/frequenz/channels/util/_select.py index 949a9347..5d72778b 100644 --- a/src/frequenz/channels/util/_select.py +++ b/src/frequenz/channels/util/_select.py @@ -9,15 +9,406 @@ """ import asyncio +import datetime import logging +from collections.abc import AsyncIterator from dataclasses import dataclass -from typing import Any, TypeVar +from typing import Any, Generic, TypeGuard, TypeVar from .._base_classes import Receiver from .._exceptions import ReceiverStoppedError logger = logging.Logger(__name__) -T = TypeVar("T") +_T = TypeVar("_T") + + +class Selected(Generic[_T]): + """The result of a [`select()`][frequenz.channels.util.select] operation. + + This class is used as the result of a select operation. The selected receiver is + consumed immediately and the received value is stored in the instance, unless there + was an exception while receiving the value, in which case the exception is stored + instead. + + `Selected` instances should be used in conjunction with the + [`selected_from()`][frequenz.channels.util.selected_from] function to determine + which receiver was selected. + + Please see [`select()`][frequenz.channels.util.select] for an example. + """ + + class _EmptyResult: + def __repr__(self) -> str: + return "" + + def __init__(self, receiver: Receiver[_T]) -> None: + """Create a new instance. + + The receiver is consumed immediately when creating the instance and the received + value is stored in the instance for later use as `value`. If there was an + exception while receiving the value, then the exception is stored in the + instance instead (as `exception`). + + Args: + receiver: The receiver that was selected. + """ + self._recv: Receiver[_T] = receiver + """The receiver that was selected.""" + + # We need a sentinel value to distinguish between None and empty result + # because a result can be None + self._value: _T | Selected._EmptyResult = Selected._EmptyResult() + """The value that was received. + + If there was an exception while receiving the value, then this will be `None`. + """ + self._exception: Exception | None = None + """The exception that was raised while receiving the value (if any).""" + + try: + self._value = receiver.consume() + except Exception as exc: # pylint: disable=broad-except + self._exception = exc + + self._handled: bool = False + """Flag to indicate if this selected has been handled in the if-chain.""" + + @property + def value(self) -> _T: + """The value that was received, if any. + + Returns: + The value that was received. + + Raises: + Exception: If there was an exception while receiving the value. Normally + this should be an [`frequenz.channels.Error`][frequenz.channels.Error] + instance, but catches all exceptions in case some receivers can raise + anything else. + + # noqa: DAR401 _exception + """ + if self._exception is not None: + raise self._exception + assert not isinstance(self._value, Selected._EmptyResult) + return self._value + + @property + def exception(self) -> Exception | None: + """The exception that was raised while receiving the value (if any). + + Returns: + The exception that was raised while receiving the value (if any). + """ + return self._exception + + @property + def was_stopped(self) -> bool: + """Whether the receiver was stopped. + + A receiver is considered stopped if it raised a `ReceiverStoppedError` while + receiving a value. + + Returns: + Whether the receiver was stopped. + """ + return isinstance(self._exception, ReceiverStoppedError) + + def __str__(self) -> str: + """Return a string representation of the instance. + + Returns: + A string representation of the instance. + """ + return ( + f"{type(self).__name__}({self._recv}) -> " + f"{self._exception or self._value})" + ) + + def __repr__(self) -> str: + """Return a the internal representation of the instance. + + Returns: + A string representation of the instance. + """ + return ( + f"{type(self).__name__}({self._recv=}, {self._value=}, " + f"{self._exception=}, {self._handled=})" + ) + + +# It would have been nice to be able to make this a method of selected, but sadly +# `TypeGuard`s can't be used as methods. For more information see: +# https://github.com/microsoft/pyright/discussions/3125 +def selected_from( + selected: Selected[Any], receiver: Receiver[_T] +) -> TypeGuard[Selected[_T]]: + """Check if the given receiver was [`select()`][frequenz.channels.util.select]ed. + + This function is used in conjunction with the `Selected` class to determine which + receiver was selected in a select operation. + + It also works as a type guard to narrow the type of the `Selected` instance to the + type of the receiver. + + Please see [`select()`][frequenz.channels.util.select] for an example. + + Args: + selected: The result of a select operation. + receiver: The receiver to check. + + Returns: + Whether the given receiver was selected. + """ + if handled := selected._recv is receiver: # pylint: disable=protected-access + selected._handled = True # pylint: disable=protected-access + return handled + + +class SelectError(BaseException): + """A base exception for [`select()`][frequenz.channels.util.select] operations. + + This exception is raised when a select operation fails. It is raised as a single + exception when one receiver fails during normal operation (while calling `ready()` + for example). It is raised as a group exception + ([`SelectErrorGroup`][frequenz.channels.util.SelectErrorGroup]) when multiple + receivers fail at the same time (while cleaning up for example). + """ + + +class UnhandledSelectedError(SelectError, Generic[_T]): + """A receiver was not handled in [`select()`][frequenz.channels.util.select]. + + This exception is raised when a select loop finishes without a call to + [`selected_from()`][frequenz.channels.util.selected_from] for the selected receiver. + """ + + def __init__(self, selected: Selected[_T]) -> None: + """Create a new instance. + + Args: + selected: The selected receiver that was not handled. + """ + recv = selected._recv # pylint: disable=protected-access + super().__init__(f"Selected receiver {recv} was not handled in the if-chain") + self.selected = selected + + +class SelectErrorGroup(BaseExceptionGroup[BaseException], SelectError): + """An exception group for [`select()`][frequenz.channels.util.select] operations. + + This exception group is raised when a select operation fails while cleaning up, so + many receivers could fail at the same time. + """ + + +# Typing for select() is tricky. We had the idea of using a declarative design for +# select, something like: +# +# ```python +# class MySelector(Selector): +# receiver1: x.new_receiver() +# receiver2: y.new_receiver() +# +# async for selected in MySelector: +# if selected.receiver is receiver1: +# # Do something with selected.value +# elif selected.receiver is receiver1: +# # Do something with selected.value +# ``` +# +# This is similar to `Enum`, but `Enum` has special support in `mypy` that we can't +# have. +# +# With the current implementation, the typing could be slightly improved by using +# `TypeVarTuple`, but we are not because "transformations" are not supported yet, see: +# https://github.com/python/typing/issues/1216 +# +# Also support for `TypeVarTuple` in general is still experimental (and very incomplete +# in `mypy`). +# +# With this we would also probably be able to properly type `select` and *maybe* even be +# able to leverage the exhaustiveness checking of `mypy` to make sure the selected value +# is narrowed down to the correct type to make sure all receivers are handled, with the +# help of `assert_never` as described in: +# https://docs.python.org/3.11/library/typing.html#typing.assert_never +# +# We also explored the possibility of using `match` to perform exhaustiveness checking, +# but we couldn't find a way to make it work with `match`, and `match` is not yet +# checked for exhaustiveness by `mypy` anyway, see: +# https://github.com/python/mypy/issues/13597 +async def select( + *receivers: Receiver[Any], timeout: datetime.timedelta | None = None +) -> AsyncIterator[Selected[Any]]: + """Iterate over the values of all receivers as they receive new values. + + This function is used to iterate over the values of all receivers as they receive + new values. It is used in conjunction with the + [`Selected`][frequenz.channels.util.Selected] class and the + [`selected_from()`][frequenz.channels.util.selected_from] function to determine + which function to determine which receiver was selected in a select operation. + + An exhaustiveness check is performed at runtime to make sure all selected receivers + are handled in the if-chain, so you should call `selected_from()` with all the + receivers passed to `select()` inside the select loop, even if you plan to ignore + a value, to signal `select()` that you are purposefully ignoring the value. + + Note: + The `select()` function is intended to be used in cases where the set of + receivers is static and known beforehand. If you need to dynamically add/remove + receivers from a select loop, there are a few alternatives. Depending on your + use case, one or the other could work better for you: + + * Use [`Merge`][frequenz.channels.util.Merge] or + [`MergeNamed`][frequenz.channels.util.MergeNamed]: this is useful when you + have and unknown number of receivers of the same type that can be handled as + a group. + * Use tasks to manage each recever individually: this is better if there are no + relationships between the receivers. + * Break the `select()` loop and start a new one with the new set of receivers + (this should be the last resort, as it has some performance implications + because the loop needs to be restarted). + + Example: + ```python + from typing import assert_never + + from frequenz.channels import ReceiverStoppedError + from frequenz.channels.util import select, selected_from, Timer + + timer1 = Timer.periodic(datetime.timedelta(seconds=1)) + timer2 = Timer.timeout(datetime.timedelta(seconds=0.5)) + + async for selected in select(timer1, timer2): + if selected_from(selected, timer1): + # Beware: `selected.value` might raise an exception, you can always + # check for exceptions with `selected.exception` first or use + # a try-except block. You can also quickly check if the receiver was + # stopped and let any other unexpected exceptions bubble up. + if selected.was_stopped: + print("timer1 was stopped") + continue + print(f"timer1: now={datetime.datetime.now()} drift={selected.value}") + timer2.stop() + elif selected_from(selected, timer2): + # Explicitly handling of exceptions + match selected.exception: + case ReceiverStoppedError(): + print("timer2 was stopped") + case Exception() as exception: + print(f"timer2: exception={exception}") + case None: + # All good, no exception, we can use `selected.value` safely + print( + f"timer2: now={datetime.datetime.now()} drift={selected.value}" + ) + case _ as unhanded: + assert_never(unhanded) + else: + # This is not necessary, as select() will check for exhaustiveness, but + # it is good practice to have it in case you forgot to handle a new + # receiver added to `select()` at a later point in time. + assert False + ``` + + Args: + *receivers: The receivers to select from. + timeout: The timeout for the select operation. If not `None`, the loop will + block for at most the specified time. If the timeout is reached, the + iteration will stop. + + Yields: + The currently selected item. + + Raises: + UnhandledSelectedError: If a selected receiver was not handled in the if-chain. + SelectErrorGroup: If there is an error while finishing the select operation and + receivers fail while cleaning up. + SelectError: If there is an error while selecting receivers during normal + operation. For example if a receiver raises an exception in the `ready()` + method. Normal errors while receiving values are not raised, but reported + via the `Selected` instance. + """ + receivers_map: dict[str, Receiver[Any]] = {str(hash(r)): r for r in receivers} + pending: set[asyncio.Task[bool]] = set() + + try: + for name, recv in receivers_map.items(): + pending.add(asyncio.create_task(_recv_task(recv), name=name)) + + while pending: + done, pending = await asyncio.wait( + pending, + return_when=asyncio.FIRST_COMPLETED, + timeout=timeout.total_seconds() if timeout else None, + ) + + # If the timeout is reached, then we return immediately + if not done and timeout: + return + + for task in done: + name = task.get_name() + recv = receivers_map[name] + if exception := task.exception(): + raise SelectError(f"Error while selecting {recv}") from exception + + selected = Selected(recv) + yield selected + if not selected._handled: # pylint: disable=protected-access + raise UnhandledSelectedError(selected) + + receiver_active = task.result() + if not receiver_active: + continue + + # Add back the receiver to the pending list + name = task.get_name() + recv = receivers_map[name] + pending.add(asyncio.create_task(_recv_task(recv), name=name)) + finally: + await _stop_pending_tasks(pending) + + +async def _recv_task(recv: Receiver[Any]) -> bool: + """Wait for a receiver to be ready catching `CancelledError` exceptions. + + It catches `CancelledError` exceptions and returns `False` to indicate that the + receiver is no longer active. + + Args: + recv: The receiver to check. + + Returns: + Whether the receiver is still active. + """ + try: + return await recv.ready() + except asyncio.CancelledError: + return False + + +async def _stop_pending_tasks(pending: set[asyncio.Task[bool]]) -> None: + """Stop all pending tasks. + + Args: + pending: The pending tasks to stop. + + Raises: + SelectErrorGroup: If the receivers raise any exceptions. + """ + if pending: + for task in pending: + task.cancel() + done, pending = await asyncio.wait(pending) + assert not pending + exceptions: list[BaseException] = [] + for task in done: + if exception := task.exception(): + if not isinstance(exception, asyncio.CancelledError): + exceptions.append(exception) + if exceptions: + raise SelectErrorGroup("Some receivers failed when select()ing", exceptions) @dataclass From b000713c4c50d2ea40daff6598562b43a99730db Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Tue, 13 Jun 2023 13:56:30 +0200 Subject: [PATCH 06/20] Add a Event utility receiver This receiver can be manually made ready. It is mainly useful for testing but can also become handy in scenarios where a simple, on-off signal needs to be sent to a select loop for example. Signed-off-by: Leandro Lucarella --- src/frequenz/channels/util/__init__.py | 5 + src/frequenz/channels/util/_event.py | 161 +++++++++++++++++++++++++ tests/utils/test_event.py | 59 +++++++++ 3 files changed, 225 insertions(+) create mode 100644 src/frequenz/channels/util/_event.py create mode 100644 tests/utils/test_event.py diff --git a/src/frequenz/channels/util/__init__.py b/src/frequenz/channels/util/__init__.py index d9f5a29f..34cdcf7f 100644 --- a/src/frequenz/channels/util/__init__.py +++ b/src/frequenz/channels/util/__init__.py @@ -5,6 +5,9 @@ A module with several utilities to work with channels: +* [Event][frequenz.channels.util.Event]: + A [receiver][frequenz.channels.Receiver] that can be made ready through an event. + * [FileWatcher][frequenz.channels.util.FileWatcher]: A [receiver][frequenz.channels.Receiver] that watches for file events. @@ -28,6 +31,7 @@ of [receivers][frequenz.channels.Receiver] and select the next available value. """ +from ._event import Event from ._file_watcher import FileWatcher from ._merge import Merge from ._merge_named import MergeNamed @@ -49,6 +53,7 @@ ) __all__ = [ + "Event", "FileWatcher", "Merge", "MergeNamed", diff --git a/src/frequenz/channels/util/_event.py b/src/frequenz/channels/util/_event.py new file mode 100644 index 00000000..3c24a8ee --- /dev/null +++ b/src/frequenz/channels/util/_event.py @@ -0,0 +1,161 @@ +# License: MIT +# Copyright © 2023 Frequenz Energy-as-a-Service GmbH + +"""A receiver that can be made ready through an event.""" + + +import asyncio as _asyncio + +from frequenz.channels import _base_classes, _exceptions + + +class Event(_base_classes.Receiver[None]): + """A receiver that can be made ready through an event. + + The receiver (the [`ready()`][frequenz.channels.util.Event.ready] method) will wait + until [`set()`][frequenz.channels.util.Event.set] is called. At that point the + receiver will wait again after the event is + [`consume()`][frequenz.channels.Receiver.consume]d. + + The receiver can be completely stopped by calling + [`stop()`][frequenz.channels.Receiver.stop]. + + Example: + ```python + import asyncio + from frequenz.channels import Receiver + from frequenz.channels.util import Event, select, selected_from + + other_receiver: Receiver[int] = ... + exit_event = Event() + + async def exit_after_10_seconds() -> None: + asyncio.sleep(10) + exit_event.set() + + asyncio.ensure_future(exit_after_10_seconds()) + + async for selected in select(exit_event, other_receiver): + if selected_from(selected, exit_event): + break + if selected_from(selected, other_receiver): + print(selected.value) + else: + assert False, "Unknow receiver selected" + ``` + """ + + def __init__(self, name: str | None = None) -> None: + """Create a new instance. + + Args: + name: The name of the receiver. If `None` the `id(self)` will be used as + the name. This is only for debugging purposes, it will be shown in the + string representation of the receiver. + """ + self._event: _asyncio.Event = _asyncio.Event() + """The event that is set when the receiver is ready.""" + + self._name: str = name or str(id(self)) + """The name of the receiver. + + This is for debugging purposes, it will be shown in the string representation + of the receiver. + """ + + self._is_set: bool = False + """Whether the receiver is ready to be consumed. + + This is used to differentiate between when the receiver was stopped (the event + is triggered too) but still there is an event to be consumed and when it was + stopped but was not explicitly set(). + """ + + self._is_stopped: bool = False + """Whether the receiver is stopped.""" + + @property + def name(self) -> str: + """The name of this receiver. + + This is for debugging purposes, it will be shown in the string representation + of this receiver. + + Returns: + The name of this receiver. + """ + return self._name + + @property + def is_set(self) -> bool: + """Whether this receiver is set (ready). + + Returns: + Whether this receiver is set (ready). + """ + return self._is_set + + @property + def is_stopped(self) -> bool: + """Whether this receiver is stopped. + + Returns: + Whether this receiver is stopped. + """ + return self._is_stopped + + def stop(self) -> None: + """Stop this receiver.""" + self._is_stopped = True + self._event.set() + + def set(self) -> None: + """Trigger the event (make the receiver ready).""" + self._is_set = True + self._event.set() + + async def ready(self) -> bool: + """Wait until this receiver is ready. + + Returns: + Whether this receiver is still running. + """ + if self._is_stopped: + return False + await self._event.wait() + return not self._is_stopped + + def consume(self) -> None: + """Consume the event. + + This makes this receiver wait again until the event is set again. + + Raises: + ReceiverStoppedError: If this receiver is stopped. + """ + if not self._is_set and self._is_stopped: + raise _exceptions.ReceiverStoppedError(self) + + assert self._is_set, "calls to `consume()` must be follow a call to `ready()`" + + self._is_set = False + self._event.clear() + + def __str__(self) -> str: + """Return a string representation of this receiver. + + Returns: + A string representation of this receiver. + """ + return f"{type(self).__name__}({self._name!r})" + + def __repr__(self) -> str: + """Return a string representation of this receiver. + + Returns: + A string representation of this receiver. + """ + return ( + f"<{type(self).__name__} name={self._name!r} is_set={self.is_set!r} " + f"is_stopped={self.is_stopped!r}>" + ) diff --git a/tests/utils/test_event.py b/tests/utils/test_event.py new file mode 100644 index 00000000..0cda9d23 --- /dev/null +++ b/tests/utils/test_event.py @@ -0,0 +1,59 @@ +# License: MIT +# Copyright © 2023 Frequenz Energy-as-a-Service GmbH + +"""Tests for the select implementation.""" + +import asyncio as _asyncio + +import pytest as _pytest + +from frequenz.channels import ReceiverStoppedError +from frequenz.channels.util import Event + + +async def test_event() -> None: + """Test the event implementation.""" + event = Event() + assert not event.is_set + assert not event.is_stopped + + is_ready = False + + async def wait_for_event() -> None: + nonlocal is_ready + await event.ready() + is_ready = True + + event_task = _asyncio.create_task(wait_for_event()) + + await _asyncio.sleep(0) # Yield so the wait_for_event task can run. + + assert not is_ready + assert not event.is_set + assert not event.is_stopped + + event.set() + + await _asyncio.sleep(0) # Yield so the wait_for_event task can run. + assert is_ready + assert event.is_set + assert not event.is_stopped + + event.consume() + assert not event.is_set + assert not event.is_stopped + assert event_task.done() + assert event_task.result() is None + assert not event_task.cancelled() + + event.stop() + assert not event.is_set + assert event.is_stopped + + await event.ready() + with _pytest.raises(ReceiverStoppedError): + event.consume() + assert event.is_stopped + assert not event.is_set + + await event_task From 9015e7da0f9faeb2bae470d62c040f9ea1839ceb Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 19 Jun 2023 14:33:53 +0200 Subject: [PATCH 07/20] Revert "Add a new async iterable select() function" Sadly the generator approach doesn't work because of limitations in how Python handles the `yield` statement and breaking out from a `async for` for loop (actually this applies to sync generators too). When the loop is broken, the control is never passed back to the async genreator, so the `finally` block is never executed (at least not until the end of the program, where dangling generators are cleared). Because of this, we will need to use a class instead, and to make it easy to guarantee avoiding leaking resources (tasks), we make it an async context managaer and an async iterator. This reverts commit 2764d0171d926bc2e41c29eca45a719fcc797f3f. Example: ```python import asyncio from typing import AsyncIterator was_gen_finalized: bool = False async def gen() -> AsyncIterator[int]: try: print("gen") yield 1 finally: global was_gen_finalized was_gen_finalized = True print("gen finally") async def main() -> None: global was_gen_finalized print("1. without break") async for i in gen(): print(i) print(f" end of loop: {was_gen_finalized=}") was_gen_finalized = False print("------------------") print("2. with break") async for i in gen(): print(i) break print(f" end of loop: {was_gen_finalized=}") was_gen_finalized = False print("------------------") print("2. with exception") try: async for i in gen(): print(i) raise Exception("Interrupted by exception") except Exception: pass print(f" end of loop: {was_gen_finalized=}") was_gen_finalized = False print() print("END of main") asyncio.run(main()) print(f"END of asyncio.run(): {was_gen_finalized=}") ``` This program prints: ``` 1. without break gen 1 gen finally end of loop: was_gen_finalized=True ------------------ 2. with break gen 1 end of loop: was_gen_finalized=False ------------------ 2. with exception gen 1 end of loop: was_gen_finalized=False END of main gen finally gen finally END of asyncio.run(): was_gen_finalized=True ``` Signed-off-by: Leandro Lucarella --- src/frequenz/channels/util/__init__.py | 21 +- src/frequenz/channels/util/_select.py | 395 +------------------------ 2 files changed, 4 insertions(+), 412 deletions(-) diff --git a/src/frequenz/channels/util/__init__.py b/src/frequenz/channels/util/__init__.py index 34cdcf7f..eba4cc28 100644 --- a/src/frequenz/channels/util/__init__.py +++ b/src/frequenz/channels/util/__init__.py @@ -26,24 +26,13 @@ * [Select][frequenz.channels.util.Select]: A helper to select the next available message for each [receiver][frequenz.channels.Receiver] in a group of receivers. - -* [select][frequenz.channels.util.select]: A function to iterate over a group - of [receivers][frequenz.channels.Receiver] and select the next available value. """ from ._event import Event from ._file_watcher import FileWatcher from ._merge import Merge from ._merge_named import MergeNamed -from ._select import ( - Select, - Selected, - SelectError, - SelectErrorGroup, - UnhandledSelectedError, - select, - selected_from, -) +from ._select import Select from ._timer import ( MissedTickPolicy, SkipMissedAndDrift, @@ -58,15 +47,9 @@ "Merge", "MergeNamed", "MissedTickPolicy", + "Timer", "Select", - "SelectError", - "SelectErrorGroup", - "Selected", "SkipMissedAndDrift", "SkipMissedAndResync", - "Timer", "TriggerAllMissed", - "UnhandledSelectedError", - "select", - "selected_from", ] diff --git a/src/frequenz/channels/util/_select.py b/src/frequenz/channels/util/_select.py index 5d72778b..949a9347 100644 --- a/src/frequenz/channels/util/_select.py +++ b/src/frequenz/channels/util/_select.py @@ -9,406 +9,15 @@ """ import asyncio -import datetime import logging -from collections.abc import AsyncIterator from dataclasses import dataclass -from typing import Any, Generic, TypeGuard, TypeVar +from typing import Any, TypeVar from .._base_classes import Receiver from .._exceptions import ReceiverStoppedError logger = logging.Logger(__name__) -_T = TypeVar("_T") - - -class Selected(Generic[_T]): - """The result of a [`select()`][frequenz.channels.util.select] operation. - - This class is used as the result of a select operation. The selected receiver is - consumed immediately and the received value is stored in the instance, unless there - was an exception while receiving the value, in which case the exception is stored - instead. - - `Selected` instances should be used in conjunction with the - [`selected_from()`][frequenz.channels.util.selected_from] function to determine - which receiver was selected. - - Please see [`select()`][frequenz.channels.util.select] for an example. - """ - - class _EmptyResult: - def __repr__(self) -> str: - return "" - - def __init__(self, receiver: Receiver[_T]) -> None: - """Create a new instance. - - The receiver is consumed immediately when creating the instance and the received - value is stored in the instance for later use as `value`. If there was an - exception while receiving the value, then the exception is stored in the - instance instead (as `exception`). - - Args: - receiver: The receiver that was selected. - """ - self._recv: Receiver[_T] = receiver - """The receiver that was selected.""" - - # We need a sentinel value to distinguish between None and empty result - # because a result can be None - self._value: _T | Selected._EmptyResult = Selected._EmptyResult() - """The value that was received. - - If there was an exception while receiving the value, then this will be `None`. - """ - self._exception: Exception | None = None - """The exception that was raised while receiving the value (if any).""" - - try: - self._value = receiver.consume() - except Exception as exc: # pylint: disable=broad-except - self._exception = exc - - self._handled: bool = False - """Flag to indicate if this selected has been handled in the if-chain.""" - - @property - def value(self) -> _T: - """The value that was received, if any. - - Returns: - The value that was received. - - Raises: - Exception: If there was an exception while receiving the value. Normally - this should be an [`frequenz.channels.Error`][frequenz.channels.Error] - instance, but catches all exceptions in case some receivers can raise - anything else. - - # noqa: DAR401 _exception - """ - if self._exception is not None: - raise self._exception - assert not isinstance(self._value, Selected._EmptyResult) - return self._value - - @property - def exception(self) -> Exception | None: - """The exception that was raised while receiving the value (if any). - - Returns: - The exception that was raised while receiving the value (if any). - """ - return self._exception - - @property - def was_stopped(self) -> bool: - """Whether the receiver was stopped. - - A receiver is considered stopped if it raised a `ReceiverStoppedError` while - receiving a value. - - Returns: - Whether the receiver was stopped. - """ - return isinstance(self._exception, ReceiverStoppedError) - - def __str__(self) -> str: - """Return a string representation of the instance. - - Returns: - A string representation of the instance. - """ - return ( - f"{type(self).__name__}({self._recv}) -> " - f"{self._exception or self._value})" - ) - - def __repr__(self) -> str: - """Return a the internal representation of the instance. - - Returns: - A string representation of the instance. - """ - return ( - f"{type(self).__name__}({self._recv=}, {self._value=}, " - f"{self._exception=}, {self._handled=})" - ) - - -# It would have been nice to be able to make this a method of selected, but sadly -# `TypeGuard`s can't be used as methods. For more information see: -# https://github.com/microsoft/pyright/discussions/3125 -def selected_from( - selected: Selected[Any], receiver: Receiver[_T] -) -> TypeGuard[Selected[_T]]: - """Check if the given receiver was [`select()`][frequenz.channels.util.select]ed. - - This function is used in conjunction with the `Selected` class to determine which - receiver was selected in a select operation. - - It also works as a type guard to narrow the type of the `Selected` instance to the - type of the receiver. - - Please see [`select()`][frequenz.channels.util.select] for an example. - - Args: - selected: The result of a select operation. - receiver: The receiver to check. - - Returns: - Whether the given receiver was selected. - """ - if handled := selected._recv is receiver: # pylint: disable=protected-access - selected._handled = True # pylint: disable=protected-access - return handled - - -class SelectError(BaseException): - """A base exception for [`select()`][frequenz.channels.util.select] operations. - - This exception is raised when a select operation fails. It is raised as a single - exception when one receiver fails during normal operation (while calling `ready()` - for example). It is raised as a group exception - ([`SelectErrorGroup`][frequenz.channels.util.SelectErrorGroup]) when multiple - receivers fail at the same time (while cleaning up for example). - """ - - -class UnhandledSelectedError(SelectError, Generic[_T]): - """A receiver was not handled in [`select()`][frequenz.channels.util.select]. - - This exception is raised when a select loop finishes without a call to - [`selected_from()`][frequenz.channels.util.selected_from] for the selected receiver. - """ - - def __init__(self, selected: Selected[_T]) -> None: - """Create a new instance. - - Args: - selected: The selected receiver that was not handled. - """ - recv = selected._recv # pylint: disable=protected-access - super().__init__(f"Selected receiver {recv} was not handled in the if-chain") - self.selected = selected - - -class SelectErrorGroup(BaseExceptionGroup[BaseException], SelectError): - """An exception group for [`select()`][frequenz.channels.util.select] operations. - - This exception group is raised when a select operation fails while cleaning up, so - many receivers could fail at the same time. - """ - - -# Typing for select() is tricky. We had the idea of using a declarative design for -# select, something like: -# -# ```python -# class MySelector(Selector): -# receiver1: x.new_receiver() -# receiver2: y.new_receiver() -# -# async for selected in MySelector: -# if selected.receiver is receiver1: -# # Do something with selected.value -# elif selected.receiver is receiver1: -# # Do something with selected.value -# ``` -# -# This is similar to `Enum`, but `Enum` has special support in `mypy` that we can't -# have. -# -# With the current implementation, the typing could be slightly improved by using -# `TypeVarTuple`, but we are not because "transformations" are not supported yet, see: -# https://github.com/python/typing/issues/1216 -# -# Also support for `TypeVarTuple` in general is still experimental (and very incomplete -# in `mypy`). -# -# With this we would also probably be able to properly type `select` and *maybe* even be -# able to leverage the exhaustiveness checking of `mypy` to make sure the selected value -# is narrowed down to the correct type to make sure all receivers are handled, with the -# help of `assert_never` as described in: -# https://docs.python.org/3.11/library/typing.html#typing.assert_never -# -# We also explored the possibility of using `match` to perform exhaustiveness checking, -# but we couldn't find a way to make it work with `match`, and `match` is not yet -# checked for exhaustiveness by `mypy` anyway, see: -# https://github.com/python/mypy/issues/13597 -async def select( - *receivers: Receiver[Any], timeout: datetime.timedelta | None = None -) -> AsyncIterator[Selected[Any]]: - """Iterate over the values of all receivers as they receive new values. - - This function is used to iterate over the values of all receivers as they receive - new values. It is used in conjunction with the - [`Selected`][frequenz.channels.util.Selected] class and the - [`selected_from()`][frequenz.channels.util.selected_from] function to determine - which function to determine which receiver was selected in a select operation. - - An exhaustiveness check is performed at runtime to make sure all selected receivers - are handled in the if-chain, so you should call `selected_from()` with all the - receivers passed to `select()` inside the select loop, even if you plan to ignore - a value, to signal `select()` that you are purposefully ignoring the value. - - Note: - The `select()` function is intended to be used in cases where the set of - receivers is static and known beforehand. If you need to dynamically add/remove - receivers from a select loop, there are a few alternatives. Depending on your - use case, one or the other could work better for you: - - * Use [`Merge`][frequenz.channels.util.Merge] or - [`MergeNamed`][frequenz.channels.util.MergeNamed]: this is useful when you - have and unknown number of receivers of the same type that can be handled as - a group. - * Use tasks to manage each recever individually: this is better if there are no - relationships between the receivers. - * Break the `select()` loop and start a new one with the new set of receivers - (this should be the last resort, as it has some performance implications - because the loop needs to be restarted). - - Example: - ```python - from typing import assert_never - - from frequenz.channels import ReceiverStoppedError - from frequenz.channels.util import select, selected_from, Timer - - timer1 = Timer.periodic(datetime.timedelta(seconds=1)) - timer2 = Timer.timeout(datetime.timedelta(seconds=0.5)) - - async for selected in select(timer1, timer2): - if selected_from(selected, timer1): - # Beware: `selected.value` might raise an exception, you can always - # check for exceptions with `selected.exception` first or use - # a try-except block. You can also quickly check if the receiver was - # stopped and let any other unexpected exceptions bubble up. - if selected.was_stopped: - print("timer1 was stopped") - continue - print(f"timer1: now={datetime.datetime.now()} drift={selected.value}") - timer2.stop() - elif selected_from(selected, timer2): - # Explicitly handling of exceptions - match selected.exception: - case ReceiverStoppedError(): - print("timer2 was stopped") - case Exception() as exception: - print(f"timer2: exception={exception}") - case None: - # All good, no exception, we can use `selected.value` safely - print( - f"timer2: now={datetime.datetime.now()} drift={selected.value}" - ) - case _ as unhanded: - assert_never(unhanded) - else: - # This is not necessary, as select() will check for exhaustiveness, but - # it is good practice to have it in case you forgot to handle a new - # receiver added to `select()` at a later point in time. - assert False - ``` - - Args: - *receivers: The receivers to select from. - timeout: The timeout for the select operation. If not `None`, the loop will - block for at most the specified time. If the timeout is reached, the - iteration will stop. - - Yields: - The currently selected item. - - Raises: - UnhandledSelectedError: If a selected receiver was not handled in the if-chain. - SelectErrorGroup: If there is an error while finishing the select operation and - receivers fail while cleaning up. - SelectError: If there is an error while selecting receivers during normal - operation. For example if a receiver raises an exception in the `ready()` - method. Normal errors while receiving values are not raised, but reported - via the `Selected` instance. - """ - receivers_map: dict[str, Receiver[Any]] = {str(hash(r)): r for r in receivers} - pending: set[asyncio.Task[bool]] = set() - - try: - for name, recv in receivers_map.items(): - pending.add(asyncio.create_task(_recv_task(recv), name=name)) - - while pending: - done, pending = await asyncio.wait( - pending, - return_when=asyncio.FIRST_COMPLETED, - timeout=timeout.total_seconds() if timeout else None, - ) - - # If the timeout is reached, then we return immediately - if not done and timeout: - return - - for task in done: - name = task.get_name() - recv = receivers_map[name] - if exception := task.exception(): - raise SelectError(f"Error while selecting {recv}") from exception - - selected = Selected(recv) - yield selected - if not selected._handled: # pylint: disable=protected-access - raise UnhandledSelectedError(selected) - - receiver_active = task.result() - if not receiver_active: - continue - - # Add back the receiver to the pending list - name = task.get_name() - recv = receivers_map[name] - pending.add(asyncio.create_task(_recv_task(recv), name=name)) - finally: - await _stop_pending_tasks(pending) - - -async def _recv_task(recv: Receiver[Any]) -> bool: - """Wait for a receiver to be ready catching `CancelledError` exceptions. - - It catches `CancelledError` exceptions and returns `False` to indicate that the - receiver is no longer active. - - Args: - recv: The receiver to check. - - Returns: - Whether the receiver is still active. - """ - try: - return await recv.ready() - except asyncio.CancelledError: - return False - - -async def _stop_pending_tasks(pending: set[asyncio.Task[bool]]) -> None: - """Stop all pending tasks. - - Args: - pending: The pending tasks to stop. - - Raises: - SelectErrorGroup: If the receivers raise any exceptions. - """ - if pending: - for task in pending: - task.cancel() - done, pending = await asyncio.wait(pending) - assert not pending - exceptions: list[BaseException] = [] - for task in done: - if exception := task.exception(): - if not isinstance(exception, asyncio.CancelledError): - exceptions.append(exception) - if exceptions: - raise SelectErrorGroup("Some receivers failed when select()ing", exceptions) +T = TypeVar("T") @dataclass From dadd2316d10b7dbeee4cda8b33dfe0fdb43f9825 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 19 Jun 2023 14:44:36 +0200 Subject: [PATCH 08/20] Add a new Selector class This new class acts as an async context manager and an async iterator, and makes sure no dangling tasks are left behind after a select loop is done. This class (as the failed `select()` function) is also designed to replace the current `Select` implementation with the following improvements: * Proper type hinting by using the new helper type guard `selected_from()`. * Fixes potential starvation issues. * Simplifies the interface by providing values one-by-one. * Guarantees there are no dangling tasks left behind when used as an async context manager. Signed-off-by: Leandro Lucarella --- src/frequenz/channels/util/__init__.py | 19 +- src/frequenz/channels/util/_select.py | 589 ++++++++++++++++++++++++- 2 files changed, 605 insertions(+), 3 deletions(-) diff --git a/src/frequenz/channels/util/__init__.py b/src/frequenz/channels/util/__init__.py index eba4cc28..0ea3bbe9 100644 --- a/src/frequenz/channels/util/__init__.py +++ b/src/frequenz/channels/util/__init__.py @@ -26,13 +26,24 @@ * [Select][frequenz.channels.util.Select]: A helper to select the next available message for each [receiver][frequenz.channels.Receiver] in a group of receivers. + +* [Selector][frequenz.channels.util.Selector]: A tool to iterate over the values of all + [receivers][frequenz.channels.Receiver] as new values become available. """ from ._event import Event from ._file_watcher import FileWatcher from ._merge import Merge from ._merge_named import MergeNamed -from ._select import Select +from ._select import ( + Select, + Selected, + SelectError, + SelectErrorGroup, + Selector, + UnhandledSelectedError, + selected_from, +) from ._timer import ( MissedTickPolicy, SkipMissedAndDrift, @@ -49,7 +60,13 @@ "MissedTickPolicy", "Timer", "Select", + "SelectError", + "SelectErrorGroup", + "Selected", + "Selector", "SkipMissedAndDrift", "SkipMissedAndResync", "TriggerAllMissed", + "UnhandledSelectedError", + "selected_from", ] diff --git a/src/frequenz/channels/util/_select.py b/src/frequenz/channels/util/_select.py index 949a9347..f4940e61 100644 --- a/src/frequenz/channels/util/_select.py +++ b/src/frequenz/channels/util/_select.py @@ -11,13 +11,598 @@ import asyncio import logging from dataclasses import dataclass -from typing import Any, TypeVar +from types import TracebackType +from typing import Any, Generic, Self, TypeGuard, TypeVar, assert_never from .._base_classes import Receiver from .._exceptions import ReceiverStoppedError logger = logging.Logger(__name__) -T = TypeVar("T") +_T = TypeVar("_T") + + +class Selected(Generic[_T]): + """A result of a [`Selector`][frequenz.channels.util.Selector] loop iteration. + + The selected receiver is consumed immediately and the received value is stored in + the instance, unless there was an exception while receiving the value, in which case + the exception is stored instead. + + `Selected` instances should be used in conjunction with the + [`selected_from()`][frequenz.channels.util.selected_from] function to determine + which receiver was selected. + + Please see [`Selector`][frequenz.channels.util.Selector] for an example. + """ + + class _EmptyResult: + """A sentinel value to distinguish between None and empty result. + + We need a sentinel because a result can also be `None`. + """ + + def __repr__(self) -> str: + return "" + + def __init__(self, receiver: Receiver[_T]) -> None: + """Create a new instance. + + The receiver is consumed immediately when creating the instance and the received + value is stored in the instance for later use as + [`value`][frequenz.channels.util.Selected.value]. If there was an exception + while receiving the value, then the exception is stored in the instance instead + (as [`exception`][frequenz.channels.util.Selected.exception]). + + Args: + receiver: The receiver that was selected. + """ + self._recv: Receiver[_T] = receiver + """The receiver that was selected.""" + + self._value: _T | Selected._EmptyResult = Selected._EmptyResult() + """The value that was received. + + If there was an exception while receiving the value, then this will be `None`. + """ + self._exception: Exception | None = None + """The exception that was raised while receiving the value (if any).""" + + try: + self._value = receiver.consume() + except Exception as exc: # pylint: disable=broad-except + self._exception = exc + + self._handled: bool = False + """Flag to indicate if this selected has been handled in the if-chain.""" + + @property + def value(self) -> _T: + """The value that was received, if any. + + Returns: + The value that was received. + + Raises: + Exception: If there was an exception while receiving the value. Normally + this should be an [`frequenz.channels.Error`][frequenz.channels.Error] + instance, but catches all exceptions in case some receivers can raise + anything else. + + # noqa: DAR401 _exception + """ + if self._exception is not None: + raise self._exception + assert not isinstance(self._value, Selected._EmptyResult) + return self._value + + @property + def exception(self) -> Exception | None: + """The exception that was raised while receiving the value (if any). + + Returns: + The exception that was raised while receiving the value (if any). + """ + return self._exception + + def was_stopped(self) -> bool: + """Check if the selected receiver was stopped. + + Check if the selected receiver raised + a [`ReceiverStoppedError`][frequenz.channels.ReceiverStoppedError] while + consuming a value. + + Returns: + Whether the receiver was stopped. + """ + return isinstance(self._exception, ReceiverStoppedError) + + def __str__(self) -> str: + """Return a string representation of this instance. + + Returns: + A string representation of this instance. + """ + return ( + f"{type(self).__name__}({self._recv}) -> " + f"{self._exception or self._value})" + ) + + def __repr__(self) -> str: + """Return a the internal representation of this instance. + + Returns: + A string representation of this instance. + """ + return ( + f"{type(self).__name__}({self._recv=}, {self._value=}, " + f"{self._exception=}, {self._handled=})" + ) + + +# It would have been nice to be able to make this a method of selected, but sadly +# `TypeGuard`s can't be used as methods. For more information see: +# https://github.com/microsoft/pyright/discussions/3125 +def selected_from( + selected: Selected[Any], receiver: Receiver[_T] +) -> TypeGuard[Selected[_T]]: + """Check if the given receiver was selected by a [`Selector`][frequenz.channels.util.Selector]. + + This function is used in conjunction with the + [`Selected`][frequenz.channels.util.Selected] class to determine which receiver was + selected in a `Selector` loop iteration. + + It also works as a [type guard][typing.TypeGuard] to narrow the type of the + `Selected` instance to the type of the receiver. + + Please see [`Selector`][frequenz.channels.util.Selector] for an example. + + Args: + selected: The result of a `Selector` loop iteration. + receiver: The receiver to check if it was the source of a select operation. + + Returns: + Whether the given receiver was selected. + """ + if handled := selected._recv is receiver: # pylint: disable=protected-access + selected._handled = True # pylint: disable=protected-access + return handled + + +class SelectError(BaseException): + """A base exception for [`Selector`][frequenz.channels.util.Selector]. + + This exception is raised when a `Selector` loop iteration fails. It is raised as + a single exception when one receiver fails during normal operation (while calling + `ready()` for example). It is raised as a group exception + ([`SelectErrorGroup`][frequenz.channels.util.SelectErrorGroup]) when a `Selector` is + [`stop()`][frequenz.channels.util.Selector.stop]ed. + """ + + +class UnhandledSelectedError(SelectError, Generic[_T]): + """A receiver was not handled in a [`Selector`][frequenz.channels.util.Selector] loop. + + This exception is raised when a `Selector` loop finishes without a call to + [`selected_from()`][frequenz.channels.util.selected_from] for the selected receiver. + """ + + def __init__(self, selected: Selected[_T]) -> None: + """Create a new instance. + + Args: + selected: The selected receiver that was not handled. + """ + recv = selected._recv # pylint: disable=protected-access + super().__init__(f"Selected receiver {recv} was not handled in the if-chain") + self.selected = selected + + +class SelectErrorGroup(BaseExceptionGroup[BaseException], SelectError): + """An exception group for [`Selector`][frequenz.channels.util.Selector]. + + This exception group is raised when a [`Selector.stop()`] fails while cleaning up + runing tasts to check for ready receivers. + """ + + +# Typing for select() is tricky. We had the idea of using a declarative design for +# select, something like: +# +# ```python +# class MySelector(Selector): +# receiver1: x.new_receiver() +# receiver2: y.new_receiver() +# +# async for selected in MySelector: +# if selected.receiver is receiver1: +# # Do something with selected.value +# elif selected.receiver is receiver1: +# # Do something with selected.value +# ``` +# +# This is similar to `Enum`, but `Enum` has special support in `mypy` that we can't +# have. +# +# With the current implementation, the typing could be slightly improved by using +# `TypeVarTuple`, but we are not because "transformations" are not supported yet, see: +# https://github.com/python/typing/issues/1216 +# +# Also support for `TypeVarTuple` in general is still experimental (and very incomplete +# in `mypy`). +# +# With this we would also probably be able to properly type `select` and *maybe* even be +# able to leverage the exhaustiveness checking of `mypy` to make sure the selected value +# is narrowed down to the correct type to make sure all receivers are handled, with the +# help of `assert_never` as described in: +# https://docs.python.org/3.11/library/typing.html#typing.assert_never +# +# We also explored the possibility of using `match` to perform exhaustiveness checking, +# but we couldn't find a way to make it work with `match`, and `match` is not yet +# checked for exhaustiveness by `mypy` anyway, see: +# https://github.com/python/mypy/issues/13597 + + +class Selector: + """A tool to iterate over the values of all receivers as new values become available. + + This tool is used to iterate over the values of all receivers as they receive new + values. It is used in conjunction with the + [`Selected`][frequenz.channels.util.Selected] class and the + [`selected_from()`][frequenz.channels.util.selected_from] [type + guard][typing.TypeGuard] to determine which receiver was selected in a `Selector` + loop iteration. + + An exhaustiveness check is performed at runtime to make sure all selected receivers + are handled in the loop if-chain. You must call `selected_from()` with all the + receivers passed to the `Selector` inside the selection loop, even if you plan to + ignore a value. This is to signal the `Selector` that you are purposefully ignoring + the value and the exhaustiveness check doesn't fail. + + A `Selector` will create one task per receiver to check if it is ready to receive a + new value. To make sure tasks are properly destroyed after a selection loop, the + `Selector` class is an [async context + manager](https://docs.python.org/3/reference/datamodel.html#async-context-managers). + + When instantiated, a `Selector` [is + stopped][frequenz.channels.util.Selector.is_stopped], and task are not started until + [`start()`][frequenz.channels.util.Selector.start] is called or the async context is + entered. When the async context is exited, the `Selector` is automatically + [`stop()`][frequenz.channels.util.Selector.stop]ed. Because of this, usually you + should not call `start()` or `stop()` manually. + + Note: + A `Selector` is intended to be used in cases where the set of receivers is + static and known beforehand. If you need to dynamically add/remove receivers + from a selector, there are a few alternatives. Depending on your use case, + one or the other could work better for you: + + * Use [`Merge`][frequenz.channels.util.Merge] or + [`MergeNamed`][frequenz.channels.util.MergeNamed]: this is useful when you + have and unknown number of receivers of the same type that can be handled as + a group. + * Use tasks to manage each recever individually: this is better if there are no + relationships between the receivers. + * Break the `Selector` loop and create new instance with the new set of + receivers. This should be the last resort, as it has some performance + implications because the tasks need to be restarted. + + Example: + ```python + import datetime + from typing import assert_never + + from frequenz.channels import ReceiverStoppedError + from frequenz.channels.util import Selector, selected_from, Timer + + timer1 = Timer.periodic(datetime.timedelta(seconds=1)) + timer2 = Timer.timeout(datetime.timedelta(seconds=0.5)) + + async with Selector(timer1, timer2) as selector: + async for selected in selector: + if selected_from(selected, timer1): + # Beware: `selected.value` might raise an exception, you can always + # check for exceptions with `selected.exception` first or use + # a try-except block. You can also quickly check if the receiver was + # stopped and let any other unexpected exceptions bubble up. + if selected.was_stopped(): + print("timer1 was stopped") + continue + print(f"timer1: now={datetime.datetime.now()} drift={selected.value}") + timer2.stop() + elif selected_from(selected, timer2): + # Explicitly handling of exceptions + match selected.exception: + case ReceiverStoppedError(): + print("timer2 was stopped") + case Exception() as exception: + print(f"timer2: exception={exception}") + case None: + # All good, no exception, we can use `selected.value` safely + print( + f"timer2: now={datetime.datetime.now()} " + f"drift={selected.value}" + ) + case _ as unhanded: + assert_never(unhanded) + else: + # This is not necessary, as select() will check for exhaustiveness, but + # it is good practice to have it in case you forgot to handle a new + # receiver added to `select()` at a later point in time. + assert False + ``` + """ + + def __init__(self, *receivers: Receiver[Any]) -> None: + """Create a new instance. + + Creating a new instance of `Selector` does not start it, so no tasks will be + created until [`start()`][frequenz.channels.util.Selector.start] is called. + When a `Selector` is used as a context manager (`async with`) it will + automatically start, so in general there is no need to worry about calling + `start()` manually. + + Args: + *receivers: The receivers to select from. + """ + self._receivers_map: dict[str, Receiver[Any]] = { + str(hash(r)): r for r in receivers + } + """The map of receiver names to receivers. + + This is used to map tasks names with receivers, so that we can easily find the + receiver that was selected when a task completes. + """ + + self._pending_tasks: set[asyncio.Task[bool]] = set() + """The set of pending tasks. + + These tasks are used to wait for the receivers to be ready to receive a new + value. + + If there are no more pending and done tasks, the selector is considered to be + done/stopped. + """ + + self._done_tasks: set[asyncio.Task[bool]] = set() + """The set of done tasks. + + These are tasks that are already done, i.e. the receivers that have a ready + value to be consumed, but that haven't been selected yet. + + If there are no more pending and done tasks, the selector is considered to be + done/stopped. + """ + + self._currently_selected: Selected[Any] | None = None + """The currently selected receiver. + + This is only saved to check for exhaustiveness. + """ + + @property + def is_stopped(self) -> bool: + """Whether this selector is currently stopped. + + Returns: + Whether this selector is currently stopped. + """ + return not self._pending_tasks and not self._done_tasks + + def start(self) -> None: + """Start this selector. + + This will create a task for each receiver passed to the constructor, and + schedule them to run. If this selector is already running, this method does + nothing. + """ + if not self.is_stopped: + return + for name, recv in self._receivers_map.items(): + self._pending_tasks.add(asyncio.create_task(recv.ready(), name=name)) + + def cancel(self) -> None: + """Cancel this selector. + + This will cancel all pending tasks, but won't wait for them to complete. If + you want to wait for them to complete, you can use + [`stop()`][frequenz.channels.util.Selector.stop] instead. + + This can be used to asynchronously stop this selector. + """ + for task in self._pending_tasks: + task.cancel() + + async def stop(self) -> None: + """Stop this selector. + + This will cancel all pending tasks, and wait for them to complete. If you + don't want to wait for them to complete, you can use + [`cancel()`][frequenz.channels.util.Selector.cancel] instead. + + Raises: + SelectErrorGroup: If there is an error in any of the pending receivers. + """ + self.cancel() + if exceptions := await self._wait_for_pending_tasks(): + raise SelectErrorGroup("Some receivers failed when select()ing", exceptions) + + async def __aenter__(self) -> Self: + """Start this selector if it is not already running. + + Returns: + This selector. + + """ + self.start() + return self + + async def __aexit__( + self, + exc_type: type[BaseException] | None, + exc_val: BaseException | None, + exc_tb: TracebackType | None, + ) -> None: + """Stop this selector. + + Args: + exc_type: The type of the exception raised, if any. + exc_val: The exception raised, if any. + exc_tb: The traceback of the exception raised, if any. + """ + await self.stop() + + def __del__(self) -> None: + """Delete this `Selector`. + + Cancel all the pending tasks. + """ + self.cancel() + + def __aiter__(self) -> Self: + """Return self as an async iterator over the selected values. + + Returns: + This selector. + """ + return self + + async def __anext__(self) -> Selected[Any]: + """Iterate over the values of all receivers as they receive new values. + + Returns: + The currently selected item. + + Raises: + UnhandledSelectedError: If the previously selected receiver was not handled + (`select_from` was not called with the selected receiver) in the + if-chain. + SelectError: If there is an internal error in this selector or a receiver + raises an exception while waiting to be ready. Errors while consuming + from the receiver are not raised, but reported via the `Selected` + instance. + StopAsyncIteration: If the `Selector` was not + [`start()`][frequenz.channels.util.Selector.start]ed, all the receivers + were stopped, or [`cancel()`][frequenz.channels.util.Selector.cancel] + was called. + """ + # Check for exhaustiveness + selected = self._currently_selected + self._currently_selected = None + if selected and not selected._handled: # pylint: disable=protected-access + raise UnhandledSelectedError(selected) + + # If we already have some receivers that are done, we can select from them + # without waiting again, but tasks could be done because they were cancelled, + # so we need to check if there is actually a selected receiver. + if self._done_tasks: + if selected := self._select_next(): + return selected + + # If there are no more receivers that are done and no pending receivers either, + # we are done. + # From now on, we need to wait for some pending receivers to be ready. + while self._pending_tasks: + # If there are pending receivers, we wait for some to be ready. + self._done_tasks, self._pending_tasks = await asyncio.wait( + self._pending_tasks, + return_when=asyncio.FIRST_COMPLETED, + ) + + # At this point, **something** should be done, but tasks could be done + # because they were cancelled, so we need to check if there is actually + # a selected receiver. + assert self._done_tasks + if selected := self._select_next(): + return selected + + # If we reached this point, there are no more pending tasks and all done tasks + # were cancelled, so we are done with the loop + assert not self._done_tasks + raise StopAsyncIteration() + + def _select_next(self) -> Selected[Any] | None: + """Select the next receiver that is ready. + + This will select the next receiver that is ready from the list of done tasks, + and return it as a `Selected` instance. + + If there is no receiver that is ready (for example because all done tasks are + from receivers that were stopped or because the tasks were cancelled), this + will return `None`. + + Returns: + The selected receiver, or `None` if there is no selected receiver. + + Raises: + SelectError: If there is an internal error in this selector or a receiver + raises an exception while waiting to be ready. + """ + while self._done_tasks: + done_task = self._done_tasks.pop() + name = done_task.get_name() + recv = self._receivers_map[name] + # If the task was cancelled, we just skip it and don't add if back to the + # pending list because we are being cancelled. + if done_task.cancelled(): + print(f"Cancelled {recv}") + continue + # If there is any other exception (unexpected, as ready() should not + # raise), we re-raise it as a SelectError. + if exception := done_task.exception(): + raise SelectError(f"Error while selecting {recv}") from exception + + self._currently_selected = Selected(recv) + + receiver_active = done_task.result() # False + receiver_stopped = self._currently_selected.was_stopped() # True + if receiver_active and not receiver_stopped: + # Add back the receiver to the pending list if it is still active. + name = done_task.get_name() + recv = self._receivers_map[name] + self._pending_tasks.add(asyncio.create_task(recv.ready(), name=name)) + + return self._currently_selected + return None + + async def _wait_for_pending_tasks(self) -> list[BaseException]: + """Wait for all pending tasks to be done. + + If the pending tasks were cancelled or the underlying receiver was stopped, then + these exceptions will be ignored. Other exceptions will be returned. + + All pending and done tasks will be cleared. + + Returns: + The list of exceptions raised by the pending tasks. + """ + if not self._pending_tasks: + return [] + + self._done_tasks, self._pending_tasks = await asyncio.wait(self._pending_tasks) + assert not self._pending_tasks + exceptions: list[BaseException] = [] + while self._done_tasks: + task = self._done_tasks.pop() + if task.cancelled(): + continue + # The assignment is a workaround for a mypy bug not doing + # proper exhaustiveness checking otherwise: + # https://github.com/python/mypy/issues/12998 + exception = task.exception() + match exception: + # We ignore ReceiverStoppedError too because they will be re-raised if + # the user tries to receive from the receiver again, so there is no + # information loss, and knowing that a receiver stopped when stopping + # the select loop doesn't provide a lot of info and it might even be + # confusing as we swallow ReceiverStoppedError during normal operation + # too. + case None | ReceiverStoppedError(): + pass + case BaseException(): + exceptions.append(exception) + case _ as unhandled: + assert_never(unhandled) + return exceptions @dataclass From db850a9052ffbe04f9afd9ae0f3b920a0dbe9a54 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 16 Jun 2023 14:04:20 +0200 Subject: [PATCH 09/20] Add test for the Selector class Signed-off-by: Leandro Lucarella --- tests/utils/test_selector.py | 55 +++ tests/utils/test_selector_integration.py | 491 +++++++++++++++++++++++ 2 files changed, 546 insertions(+) create mode 100644 tests/utils/test_selector.py create mode 100644 tests/utils/test_selector_integration.py diff --git a/tests/utils/test_selector.py b/tests/utils/test_selector.py new file mode 100644 index 00000000..efd71ad8 --- /dev/null +++ b/tests/utils/test_selector.py @@ -0,0 +1,55 @@ +# License: MIT +# Copyright © 2023 Frequenz Energy-as-a-Service GmbH + +"""Tests for the Selector implementation.""" + +from unittest import mock + +import pytest + +from frequenz.channels import Receiver, ReceiverStoppedError +from frequenz.channels.util import Selected, selected_from + + +class TestSelected: + """Tests for the Selected class.""" + + def test_with_value(self) -> None: + """Test selected from a receiver with a value.""" + recv = mock.MagicMock(spec=Receiver[int]) + recv.consume.return_value = 42 + selected = Selected[int](recv) + + assert selected_from(selected, recv) + assert selected.value == 42 + assert selected.exception is None + assert not selected.was_stopped() + + def test_with_exception(self) -> None: + """Test selected from a receiver with an exception.""" + recv = mock.MagicMock(spec=Receiver[int]) + exception = Exception("test") + recv.consume.side_effect = exception + selected = Selected[int](recv) + + assert selected_from(selected, recv) + with pytest.raises(Exception, match="test"): + _ = selected.value + assert selected.exception is exception + assert not selected.was_stopped() + + def test_with_stopped(self) -> None: + """Test selected from a stopped receiver.""" + recv = mock.MagicMock(spec=Receiver[int]) + exception = ReceiverStoppedError[int](recv) + recv.consume.side_effect = exception + selected = Selected[int](recv) + + assert selected_from(selected, recv) + with pytest.raises( + ReceiverStoppedError, + match=r"Receiver was stopped", + ): + _ = selected.value + assert selected.exception is exception + assert selected.was_stopped() diff --git a/tests/utils/test_selector_integration.py b/tests/utils/test_selector_integration.py new file mode 100644 index 00000000..f604755a --- /dev/null +++ b/tests/utils/test_selector_integration.py @@ -0,0 +1,491 @@ +# License: MIT +# Copyright © 2023 Frequenz Energy-as-a-Service GmbH + +"""Integration tests for Selector class. + +These tests are actually a bit in the middle between unit and integration, because we +are using a fake loop to make the tests faster, but we are still testing more than one +class at a time. +""" + +import asyncio +from collections.abc import AsyncIterator, Iterator +from typing import Any + +import async_solipsism +import pytest + +from frequenz.channels import Receiver, ReceiverStoppedError +from frequenz.channels.util import ( + Event, + Selected, + Selector, + UnhandledSelectedError, + selected_from, +) + + +@pytest.mark.integration +class TestSelector: + """Tests for the Selector class.""" + + recv1: Event + recv2: Event + recv3: Event + selector: Selector + loop: async_solipsism.EventLoop + + @pytest.fixture(autouse=True) + def event_loop( + self, request: pytest.FixtureRequest + ) -> Iterator[async_solipsism.EventLoop]: + """Replace the loop with one that doesn't interact with the outside world.""" + loop = async_solipsism.EventLoop() + request.cls.loop = loop + yield loop + loop.close() + + @pytest.fixture() + async def start_run_ordered_sequence(self) -> AsyncIterator[asyncio.Task[None]]: + """Start the run_ordered_sequence method and wait for it to finish. + + Yields: + The task running the run_ordered_sequence method. + """ + sequence_task = asyncio.create_task(self.run_ordered_sequence()) + yield sequence_task + await sequence_task + + def setup_method(self) -> None: + """Set up the test.""" + self.recv1 = Event("recv1") + self.recv2 = Event("recv2") + self.recv3 = Event("recv3") + self.selector = Selector(self.recv1, self.recv2, self.recv3) + + def assert_received_from( + self, + selected: Selected[Any], + receiver: Receiver[None], + *, + at_time: float, + selector_stopped: bool = False, + ) -> None: + """Assert that the selected event was received from the given receiver. + + It also asserts that: + + * The receiver didn't raise an exception. + * The receiver wasn't stopped. + * The selector is still running. + * It happened at the given time. + + Args: + selected: The selected event. + receiver: The receiver from which the event was received. + at_time: The time at which the event was received. + selector_stopped: Check if the selector is stopped or running. + """ + assert selected_from(selected, receiver) + assert selected.value is None + assert selected.exception is None + assert not selected.was_stopped() + assert self.selector.is_stopped == selector_stopped + assert self.loop.time() == at_time + + def assert_receiver_stopped( + self, + selected: Selected[Any], + receiver: Receiver[None], + *, + at_time: float, + selector_stopped: bool = False, + ) -> None: + """Assert that the selected event came from a stopped receiver. + + It also asserts that: + + * The selector is still running (if `selector_stopped` is `False`). + * The selector is stopped (if `selector_stopped` is `True`). + * It happened at the given time. + + Args: + selected: The selected event. + receiver: The receiver from which the event was received. + at_time: The time at which the event was received. + selector_stopped: Check if the selector is stopped or running. + """ + assert selected_from(selected, receiver) + assert selected.was_stopped() + assert isinstance(selected.exception, ReceiverStoppedError) + assert selected.exception.receiver is receiver + assert self.selector.is_stopped == selector_stopped + assert self.loop.time() == at_time + + # We use the loop time (and the sleeps in the run_ordered_sequence method) mainly to + # ensure we are processing the events in the correct order and we are really + # following the sequence of events we expect. + + async def run_ordered_sequence(self) -> None: + """Run the sequence of events to be tested.""" + print("time = 0") + self.recv1.set() + await asyncio.sleep(1) + + print("time = 1") + self.recv2.set() + await asyncio.sleep(1) + + print("time = 2") + self.recv3.set() + await asyncio.sleep(1) + + print("time = 3") + self.recv1.set() + await asyncio.sleep(1) + + print("time = 4") + self.recv1.set() + await asyncio.sleep(1) + + print("time = 5") + self.recv3.set() + await asyncio.sleep(1) + + print("time = 6") + self.recv2.set() + await asyncio.sleep(1) + + print("time = 7") + self.recv1.stop() + await asyncio.sleep(1) + + print("time = 8") + self.recv2.set() + await asyncio.sleep(1) + + print("time = 9") + self.recv3.stop() + await asyncio.sleep(1) + + print("time = 10") + self.recv2.set() + await asyncio.sleep(1) + + print("time = 11") + self.recv2.stop() + + # pylint: disable=redefined-outer-name + async def test_selector_receives_in_order( + self, + start_run_ordered_sequence: asyncio.Task[ # pylint: disable=unused-argument + None + ], + ) -> None: + """Test that the selector receives events in the correct order.""" + assert self.selector.is_stopped + + async with self.selector: + selector_iter = aiter(self.selector) + + selected = await anext(selector_iter) + self.assert_received_from(selected, self.recv1, at_time=0) + + selected = await anext(selector_iter) + self.assert_received_from(selected, self.recv2, at_time=1) + + selected = await anext(selector_iter) + self.assert_received_from(selected, self.recv3, at_time=2) + + selected = await anext(selector_iter) + self.assert_received_from(selected, self.recv1, at_time=3) + + selected = await anext(selector_iter) + self.assert_received_from(selected, self.recv1, at_time=4) + + selected = await anext(selector_iter) + self.assert_received_from(selected, self.recv3, at_time=5) + + selected = await anext(selector_iter) + self.assert_received_from(selected, self.recv2, at_time=6) + + selected = await anext(selector_iter) + self.assert_receiver_stopped(selected, self.recv1, at_time=7) + + selected = await anext(selector_iter) + self.assert_received_from(selected, self.recv2, at_time=8) + + selected = await anext(selector_iter) + self.assert_receiver_stopped(selected, self.recv3, at_time=9) + + selected = await anext(selector_iter) + self.assert_received_from(selected, self.recv2, at_time=10) + + selected = await anext(selector_iter) + self.assert_receiver_stopped( + selected, self.recv2, at_time=11, selector_stopped=True + ) + + with pytest.raises(StopAsyncIteration): + selected = await anext(selector_iter) + assert self.selector.is_stopped + + assert len(asyncio.all_tasks()) == 1 # Only the test task should be alive + assert self.selector.is_stopped + + async def test_break( + self, + start_run_ordered_sequence: asyncio.Task[ # pylint: disable=unused-argument + None + ], + ) -> None: + """Test that break works.""" + assert self.selector.is_stopped + + async with self.selector: + selected: Selected[Any] | None = None + async for selected in self.selector: + if selected_from(selected, self.recv1): + continue + if selected_from(selected, self.recv2): + continue + if selected_from(selected, self.recv3): + break + + assert selected is not None + self.assert_received_from(selected, self.recv3, at_time=2) + + async for selected in self.selector: + if selected_from(selected, self.recv1): + continue + if selected_from(selected, self.recv2): + break + if selected_from(selected, self.recv3): + continue + + assert selected is not None + self.assert_received_from(selected, self.recv2, at_time=6) + + async for selected in self.selector: + if selected_from(selected, self.recv1): + continue + if selected_from(selected, self.recv2): + continue + if selected_from(selected, self.recv3): + break + + assert selected is not None + self.assert_receiver_stopped(selected, self.recv3, at_time=9) + + async for selected in self.selector: + if selected_from(selected, self.recv1): + break + if selected_from(selected, self.recv2): + continue + if selected_from(selected, self.recv3): + break + + self.assert_receiver_stopped( + selected, self.recv2, at_time=11, selector_stopped=True + ) + + assert len(asyncio.all_tasks()) == 1 # Only the test task should be alive + assert self.selector.is_stopped + + async def test_missed_select_from( + self, + start_run_ordered_sequence: asyncio.Task[ # pylint: disable=unused-argument + None + ], + ) -> None: + """Test that a missed `select_from` is detected.""" + assert self.selector.is_stopped + + selected: Selected[Any] | None = None + with pytest.raises(UnhandledSelectedError) as excinfo: + async with self.selector: + async for selected in self.selector: + if selected_from(selected, self.recv1): + continue + if selected_from(selected, self.recv2): + continue + + assert False, "Should not reach this point" + + assert selected is not None + assert excinfo.value.selected is selected + self.assert_received_from( + selected, self.recv3, at_time=2, selector_stopped=True + ) + + # The test task and the run_ordered_sequence tasks should still be alive + assert len(asyncio.all_tasks()) == 2 + assert start_run_ordered_sequence in asyncio.all_tasks() + assert self.selector.is_stopped + + async def test_cancel( + self, + start_run_ordered_sequence: asyncio.Task[ # pylint: disable=unused-argument + None + ], + ) -> None: + """Test that cancel works.""" + assert self.selector.is_stopped + + received: list[str] = [] + async with self.selector: + async for selected in self.selector: + if self.loop.time() == 5: + self.selector.cancel() + if selected_from(selected, self.recv1): + received.append("recv1") + continue + if selected_from(selected, self.recv2): + received.append("recv2") + continue + if selected_from(selected, self.recv3): + received.append("recv3") + continue + + assert received == ["recv1", "recv2", "recv3", "recv1", "recv1", "recv3"] + assert self.selector.is_stopped + + # The test task and the run_ordered_sequence tasks should still be alive + assert len(asyncio.all_tasks()) == 2 + assert start_run_ordered_sequence in asyncio.all_tasks() + assert self.selector.is_stopped + + async def test_manual_start_stop( + self, + start_run_ordered_sequence: asyncio.Task[ # pylint: disable=unused-argument + None + ], + ) -> None: + """Test that cancel works.""" + assert self.selector.is_stopped + + received: list[str] = [] + + self.selector.start() + assert not self.selector.is_stopped + + async for selected in self.selector: + if self.loop.time() == 5: + await self.selector.stop() + if selected_from(selected, self.recv1): + received.append("recv1") + continue + if selected_from(selected, self.recv2): + received.append("recv2") + continue + if selected_from(selected, self.recv3): + received.append("recv3") + continue + + assert received == ["recv1", "recv2", "recv3", "recv1", "recv1", "recv3"] + # The test task and the run_ordered_sequence tasks should still be alive + assert len(asyncio.all_tasks()) == 2 + assert start_run_ordered_sequence in asyncio.all_tasks() + assert self.selector.is_stopped + + @pytest.fixture() + async def start_run_multiple_ready(self) -> AsyncIterator[asyncio.Task[None]]: + """Start the run_multiple_ready method and wait for it to finish. + + Yields: + The task running the run_multiple_ready method. + """ + sequence_task = asyncio.create_task(self.run_multiple_ready()) + yield sequence_task + await sequence_task + + async def run_multiple_ready(self) -> None: + """Run a sequence of events with multiple receivers ready.""" + print("time = 0") + self.recv1.set() + self.recv2.set() + self.recv3.set() + await asyncio.sleep(1) + + print("time = 1") + self.recv2.set() + self.recv3.set() + await asyncio.sleep(1) + + print("time = 2") + self.recv1.set() + self.recv3.set() + await asyncio.sleep(1) + + print("time = 3") + self.recv1.set() + self.recv2.set() + await asyncio.sleep(1) + + print("time = 4") + + async def test_multiple_ready( + self, + start_run_multiple_ready: asyncio.Task[None], # pylint: disable=unused-argument + ) -> None: + """Test that multiple ready receviers are handled properly. + + Also test that the loop waits forever if there are no more receivers ready. + """ + assert self.selector.is_stopped + + received: set[str] = set() + last_time: float = self.loop.time() + async with self.selector: + try: + async with asyncio.timeout(15): + async for selected in self.selector: + now = self.loop.time() + if now != last_time: # Only check when there was a jump in time + match now: + case 1: + assert received == { + self.recv1.name, + self.recv2.name, + self.recv3.name, + } + case 2: + assert received == { + self.recv2.name, + self.recv3.name, + } + case 3: + assert received == { + self.recv1.name, + self.recv3.name, + } + # case 4 needs to be checked after the timeout, as there + # are no ready receivers after time == 3. + case _: + assert False, "Should not reach this point" + received.clear() + last_time = now + + if selected_from(selected, self.recv1): + received.add(self.recv1.name) + elif selected_from(selected, self.recv2): + received.add(self.recv2.name) + elif selected_from(selected, self.recv3): + received.add(self.recv3.name) + else: + assert False, "Should not reach this point" + except asyncio.TimeoutError: + assert self.loop.time() == 15 + # This happened after time == 3, but the loop never resumes becuase + # there is nothing ready, so we need to check it after the timeout. + assert received == { + self.recv1.name, + self.recv2.name, + } + # The selector is still waiting for receivers to be ready. + assert not self.selector.is_stopped + else: + assert False, "Should have timed out" + + assert len(asyncio.all_tasks()) == 1 # The test task should still be alive + assert self.selector.is_stopped From 24601fc0c85382c12570ed2b49cdf023abaa79d7 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 19 Jun 2023 13:57:43 +0200 Subject: [PATCH 10/20] Update issue template to use "selector" instead of "select" Signed-off-by: Leandro Lucarella --- .github/ISSUE_TEMPLATE/bug.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/bug.yml b/.github/ISSUE_TEMPLATE/bug.yml index d9b39289..0119e0ed 100644 --- a/.github/ISSUE_TEMPLATE/bug.yml +++ b/.github/ISSUE_TEMPLATE/bug.yml @@ -49,7 +49,7 @@ body: - Unit, integration and performance tests (part:tests) - Build script, CI, dependencies, etc. (part:tooling) - Channels, `Broadcast`, `Bidirectional`, etc. (part:channels) - - Select (part:select) + - Selector (part:selector) - Utility receivers, `Merge`, etc. (part:receivers) validations: required: true From d1c89e7c15f923c60ff16594dd891477216a87d3 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Thu, 15 Jun 2023 15:24:22 +0200 Subject: [PATCH 11/20] Update existing code to use Selector instead of Select Signed-off-by: Leandro Lucarella --- src/frequenz/channels/_anycast.py | 2 +- src/frequenz/channels/_broadcast.py | 2 +- src/frequenz/channels/util/_timer.py | 62 ++++++++++----------- tests/utils/test_integration.py | 81 ++++++++++++++-------------- 4 files changed, 73 insertions(+), 74 deletions(-) diff --git a/src/frequenz/channels/_anycast.py b/src/frequenz/channels/_anycast.py index 1d314087..623da0ac 100644 --- a/src/frequenz/channels/_anycast.py +++ b/src/frequenz/channels/_anycast.py @@ -28,7 +28,7 @@ class Anycast(Generic[T]): thread-safe. When there are multiple channel receivers, they can be awaited - simultaneously using [Select][frequenz.channels.util.Select], + simultaneously using a [Selector][frequenz.channels.util.Selector], [Merge][frequenz.channels.util.Merge] or [MergeNamed][frequenz.channels.util.MergeNamed]. diff --git a/src/frequenz/channels/_broadcast.py b/src/frequenz/channels/_broadcast.py index 55eeb382..33c51733 100644 --- a/src/frequenz/channels/_broadcast.py +++ b/src/frequenz/channels/_broadcast.py @@ -38,7 +38,7 @@ class Broadcast(Generic[T]): are thread-safe. Because of this, `Broadcast` channels are thread-safe. When there are multiple channel receivers, they can be awaited - simultaneously using [Select][frequenz.channels.util.Select], + simultaneously using a [Selector][frequenz.channels.util.Selector], [Merge][frequenz.channels.util.Merge] or [MergeNamed][frequenz.channels.util.MergeNamed]. diff --git a/src/frequenz/channels/util/_timer.py b/src/frequenz/channels/util/_timer.py index 1145d9e6..e87cdf84 100644 --- a/src/frequenz/channels/util/_timer.py +++ b/src/frequenz/channels/util/_timer.py @@ -307,37 +307,37 @@ class Timer(Receiver[timedelta]): print(f"The timer has triggered {drift=}") ``` - But you can also use [`Select`][frequenz.channels.util.Select] to combine it - with other receivers, and even start it (semi) manually: + But you can also use a [`Selector`][frequenz.channels.util.Selector] to combine + it with other receivers, and even start it (semi) manually: ```python import logging - from frequenz.channels.util import Select + from frequenz.channels.util import Selector, selected_from from frequenz.channels import Broadcast timer = Timer.timeout(timedelta(seconds=1.0), auto_start=False) chan = Broadcast[int]("input-chan") - receiver1 = chan.new_receiver() + battery_data = chan.new_receiver() timer = Timer.timeout(timedelta(seconds=1.0), auto_start=False) # Do some other initialization, the timer will start automatically if # a message is awaited (or manually via `reset()`). - select = Select(bat_1=receiver1, timer=timer) - while await select.ready(): - if msg := select.bat_1: - if val := msg.inner: - battery_soc = val - else: - logging.warning("battery channel closed") - elif drift := select.timer: - # Print some regular battery data - print(f"Battery is charged at {battery_soc}%") + async with Selector(battery_data, timer) as selector: + async for selected in selector: + if selected_from(selected, battery_data): + if selected.was_closed(): + logging.warning("battery channel closed") + continue + battery_soc = selected.value + elif selected_from(selected, timer): + # Print some regular battery data + print(f"Battery is charged at {battery_soc}%") ``` Example: Timeout example ```python import logging - from frequenz.channels.util import Select + from frequenz.channels.util import Selector, selected_from from frequenz.channels import Broadcast def process_data(data: int): @@ -349,23 +349,23 @@ def do_heavy_processing(data: int): timer = Timer.timeout(timedelta(seconds=1.0), auto_start=False) chan1 = Broadcast[int]("input-chan-1") chan2 = Broadcast[int]("input-chan-2") - receiver1 = chan1.new_receiver() - receiver2 = chan2.new_receiver() - select = Select(bat_1=receiver1, heavy_process=receiver2, timeout=timer) - while await select.ready(): - if msg := select.bat_1: - if val := msg.inner: - process_data(val) + battery_data = chan1.new_receiver() + heavy_process = chan2.new_receiver() + async with Selector(battery_data, heavy_process, timer) as selector: + async for selected in selector: + if selected_from(selected, battery_data): + if selected.was_closed(): + logging.warning("battery channel closed") + continue + process_data(selected.value) timer.reset() - else: - logging.warning("battery channel closed") - if msg := select.heavy_process: - if val := msg.inner: - do_heavy_processing(val) - else: - logging.warning("processing channel closed") - elif drift := select.timeout: - logging.warning("No data received in time") + elif selected_from(selected, heavy_process): + if selected.was_closed(): + logging.warning("processing channel closed") + continue + do_heavy_processing(selected.value) + elif selected_from(selected, timer): + logging.warning("No data received in time") ``` In this case `do_heavy_processing` might take 2 seconds, and we don't diff --git a/tests/utils/test_integration.py b/tests/utils/test_integration.py index 9aa6de41..bb3dec86 100644 --- a/tests/utils/test_integration.py +++ b/tests/utils/test_integration.py @@ -9,7 +9,7 @@ import pytest -from frequenz.channels.util import FileWatcher, Select, Timer +from frequenz.channels.util import FileWatcher, Selector, Timer, selected_from @pytest.mark.integration @@ -20,29 +20,30 @@ async def test_file_watcher(tmp_path: pathlib.Path) -> None: tmp_path: A tmp directory to run the file watcher on. Created by pytest. """ filename = tmp_path / "test-file" - file_watcher = FileWatcher(paths=[str(tmp_path)]) number_of_writes = 0 expected_number_of_writes = 3 - select = Select( - timer=Timer.timeout(timedelta(seconds=0.1)), - file_watcher=file_watcher, - ) - while await select.ready(): - if msg := select.timer: - filename.write_text(f"{msg.inner}") - elif msg := select.file_watcher: - event_type = ( - FileWatcher.EventType.CREATE - if number_of_writes == 0 - else FileWatcher.EventType.MODIFY - ) - assert msg.inner == FileWatcher.Event(type=event_type, path=filename) - number_of_writes += 1 - # After receiving a write 3 times, unsubscribe from the writes channel - if number_of_writes == expected_number_of_writes: - break + file_watcher = FileWatcher(paths=[str(tmp_path)]) + timer = Timer.timeout(timedelta(seconds=0.1)) + + async with Selector(file_watcher, timer) as selector: + async for selected in selector: + if selected_from(selected, timer): + filename.write_text(f"{selected.value}") + elif selected_from(selected, file_watcher): + event_type = ( + FileWatcher.EventType.CREATE + if number_of_writes == 0 + else FileWatcher.EventType.MODIFY + ) + assert selected.value == FileWatcher.Event( + type=event_type, path=filename + ) + number_of_writes += 1 + # After receiving a write 3 times, unsubscribe from the writes channel + if number_of_writes == expected_number_of_writes: + break assert number_of_writes == expected_number_of_writes @@ -61,12 +62,9 @@ async def test_file_watcher_deletes(tmp_path: pathlib.Path) -> None: file_watcher = FileWatcher( paths=[str(tmp_path)], event_types={FileWatcher.EventType.DELETE} ) + write_timer = Timer.timeout(timedelta(seconds=0.1)) + deletion_timer = Timer.timeout(timedelta(seconds=0.25)) - select = Select( - write_timer=Timer.timeout(timedelta(seconds=0.1)), - deletion_timer=Timer.timeout(timedelta(seconds=0.25)), - watcher=file_watcher, - ) number_of_write = 0 number_of_deletes = 0 number_of_events = 0 @@ -87,22 +85,23 @@ async def test_file_watcher_deletes(tmp_path: pathlib.Path) -> None: # W: Write # D: Delete # E: FileWatcher Event - while await select.ready(): - if msg := select.write_timer: - if number_of_write >= 2 and number_of_events == 0: - continue - filename.write_text(f"{msg.inner}") - number_of_write += 1 - elif _ := select.deletion_timer: - # Avoid removing the file twice - if not pathlib.Path(filename).is_file(): - continue - os.remove(filename) - number_of_deletes += 1 - elif _ := select.watcher: - number_of_events += 1 - if number_of_events >= 2: - break + async with Selector(file_watcher, write_timer, deletion_timer) as selector: + async for selected in selector: + if selected_from(selected, write_timer): + if number_of_write >= 2 and number_of_events == 0: + continue + filename.write_text(f"{selected.value}") + number_of_write += 1 + elif selected_from(selected, deletion_timer): + # Avoid removing the file twice + if not pathlib.Path(filename).is_file(): + continue + os.remove(filename) + number_of_deletes += 1 + elif selected_from(selected, file_watcher): + number_of_events += 1 + if number_of_events >= 2: + break assert number_of_deletes == 2 # Can be more because the watcher could take some time to trigger From 6e9303fef37daa05af58fdf719c8a9934a51e56a Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 19 Jun 2023 14:46:04 +0200 Subject: [PATCH 12/20] Remove the old Select implementation This class was replaced by the new Selector class. Signed-off-by: Leandro Lucarella --- src/frequenz/channels/util/__init__.py | 6 - src/frequenz/channels/util/_select.py | 189 ------------------------- tests/test_select.py | 76 ---------- 3 files changed, 271 deletions(-) delete mode 100644 tests/test_select.py diff --git a/src/frequenz/channels/util/__init__.py b/src/frequenz/channels/util/__init__.py index 0ea3bbe9..ca6a21a5 100644 --- a/src/frequenz/channels/util/__init__.py +++ b/src/frequenz/channels/util/__init__.py @@ -23,10 +23,6 @@ * [Timer][frequenz.channels.util.Timer]: A [receiver][frequenz.channels.Receiver] that ticks at certain intervals. -* [Select][frequenz.channels.util.Select]: A helper to select the next - available message for each [receiver][frequenz.channels.Receiver] in a group - of receivers. - * [Selector][frequenz.channels.util.Selector]: A tool to iterate over the values of all [receivers][frequenz.channels.Receiver] as new values become available. """ @@ -36,7 +32,6 @@ from ._merge import Merge from ._merge_named import MergeNamed from ._select import ( - Select, Selected, SelectError, SelectErrorGroup, @@ -59,7 +54,6 @@ "MergeNamed", "MissedTickPolicy", "Timer", - "Select", "SelectError", "SelectErrorGroup", "Selected", diff --git a/src/frequenz/channels/util/_select.py b/src/frequenz/channels/util/_select.py index f4940e61..a921a055 100644 --- a/src/frequenz/channels/util/_select.py +++ b/src/frequenz/channels/util/_select.py @@ -9,15 +9,12 @@ """ import asyncio -import logging -from dataclasses import dataclass from types import TracebackType from typing import Any, Generic, Self, TypeGuard, TypeVar, assert_never from .._base_classes import Receiver from .._exceptions import ReceiverStoppedError -logger = logging.Logger(__name__) _T = TypeVar("_T") @@ -603,189 +600,3 @@ async def _wait_for_pending_tasks(self) -> list[BaseException]: case _ as unhandled: assert_never(unhandled) return exceptions - - -@dataclass -class _Selected: - """A wrapper class for holding values in `Select`. - - Using this wrapper class allows `Select` to inform user code when a - receiver gets closed. - """ - - inner: Any - - -@dataclass -class _ReadyReceiver: - """A class for tracking receivers that have a message ready to be read. - - Used to make sure that receivers are not consumed from until messages are accessed - by user code, at which point, it will be converted into a `_Selected` object. - - When a channel has closed, `recv` should be `None`. - """ - - recv: Receiver[Any] | None - - def get(self) -> _Selected: - """Consume a message from the receiver and return a `_Selected` object. - - Returns: - An instance of `_Selected` holding a value from the receiver. - """ - if self.recv is None: - return _Selected(None) - return _Selected(self.recv.consume()) # pylint: disable=protected-access - - -class Select: - """Select the next available message from a group of Receivers. - - If `Select` was created with more `Receiver` than what are read in - the if-chain after each call to - [ready()][frequenz.channels.util.Select.ready], messages coming in the - additional receivers are dropped, and a warning message is logged. - - [Receiver][frequenz.channels.Receiver]s also function as `Receiver`. - - When Select is no longer needed, then it should be stopped using - `self.stop()` method. This would cleanup any internal pending async tasks. - - Example: - For example, if there are two receivers that you want to - simultaneously wait on, this can be done with: - - ```python - from frequenz.channels import Broadcast - - channel1 = Broadcast[int]("input-chan-1") - channel2 = Broadcast[int]("input-chan-2") - receiver1 = channel1.new_receiver() - receiver2 = channel2.new_receiver() - - select = Select(name1 = receiver1, name2 = receiver2) - while await select.ready(): - if msg := select.name1: - if val := msg.inner: - # do something with `val` - pass - else: - # handle closure of receiver. - pass - elif msg := select.name2: - # do something with `msg.inner` - pass - ``` - """ - - def __init__(self, **kwargs: Receiver[Any]) -> None: - """Create a `Select` instance. - - Args: - **kwargs: sequence of receivers - """ - self._receivers = kwargs - self._pending: set[asyncio.Task[bool]] = set() - - for name, recv in self._receivers.items(): - self._pending.add(asyncio.create_task(recv.ready(), name=name)) - - self._ready_count = 0 - self._prev_ready_count = 0 - self._result: dict[str, _ReadyReceiver | None] = { - name: None for name in self._receivers - } - - def __del__(self) -> None: - """Cleanup any pending tasks.""" - for task in self._pending: - if not task.done() and task.get_loop().is_running(): - task.cancel() - - async def stop(self) -> None: - """Stop the `Select` instance and cleanup any pending tasks.""" - for task in self._pending: - task.cancel() - await asyncio.gather(*self._pending, return_exceptions=True) - self._pending = set() - - async def ready(self) -> bool: - """Wait until there is a message in any of the receivers. - - Returns `True` if there is a message available, and `False` if all - receivers have closed. - - Returns: - Whether there are further messages or not. - """ - # This function will change radically soon - # pylint: disable=too-many-nested-blocks - if self._ready_count > 0: - if self._ready_count == self._prev_ready_count: - dropped_names: list[str] = [] - for name, value in self._result.items(): - if value is not None: - dropped_names.append(name) - if value.recv is not None: - try: - value.recv.consume() - except ReceiverStoppedError: - pass - self._result[name] = None - self._ready_count = 0 - self._prev_ready_count = 0 - logger.warning( - "Select.ready() dropped data from receiver(s): %s, " - "because no messages have been fetched since the last call to ready().", - dropped_names, - ) - else: - self._prev_ready_count = self._ready_count - return True - if len(self._pending) == 0: - return False - - # once all the pending messages have been consumed, reset the - # `_prev_ready_count` as well, and wait for new messages. - self._prev_ready_count = 0 - - done, self._pending = await asyncio.wait( - self._pending, return_when=asyncio.FIRST_COMPLETED - ) - for task in done: - name = task.get_name() - recv = self._receivers[name] - receiver_active = task.result() - if receiver_active: - ready_recv = recv - else: - ready_recv = None - self._ready_count += 1 - self._result[name] = _ReadyReceiver(ready_recv) - # if channel or Receiver is closed - # don't add a task for it again. - if not receiver_active: - continue - self._pending.add(asyncio.create_task(recv.ready(), name=name)) - return True - - def __getattr__(self, name: str) -> Any: - """Return the latest unread message from a `Receiver`, if available. - - Args: - name: Name of the channel. - - Returns: - Latest unread message for the specified `Receiver`, or `None`. - - Raises: - KeyError: when the name was not specified when creating the - `Select` instance. - """ - result = self._result[name] - if result is None: - return result - self._result[name] = None - self._ready_count -= 1 - return result.get() diff --git a/tests/test_select.py b/tests/test_select.py deleted file mode 100644 index fa372544..00000000 --- a/tests/test_select.py +++ /dev/null @@ -1,76 +0,0 @@ -# License: MIT -# Copyright © 2022 Frequenz Energy-as-a-Service GmbH - -"""Tests for the Select implementation.""" - -import asyncio -from typing import List - -from frequenz.channels import Anycast, Sender -from frequenz.channels.util import Select - - -async def test_select() -> None: - """Ensure select receives messages in order.""" - chan1 = Anycast[int]() - chan2 = Anycast[int]() - chan3 = Anycast[int]() - - async def send(ch1: Sender[int], ch2: Sender[int], ch3: Sender[int]) -> None: - for ctr in range(5): - await ch1.send(ctr + 1) - await ch2.send(ctr + 101) - await ch3.send(ctr + 201) - await chan1.close() - await ch2.send(1000) - await chan2.close() - await chan3.close() - - senders = asyncio.create_task( - send(chan1.new_sender(), chan2.new_sender(), chan3.new_sender()), - ) - select = Select( - ch1=chan1.new_receiver(), - ch2=chan2.new_receiver(), - ch3=chan3.new_receiver(), - ) - - # only check for messages from all iterators but `ch3`. - # it ensures iterators are not blocking channels in case they - # are not being read from. - results: List[int] = [] - while await select.ready(): - if item := select.ch1: - if val := item.inner: - results.append(val) - else: - results.append(-1) - elif item := select.ch2: - if val := item.inner: - results.append(val) - else: - results.append(-2) - await senders - - expected_results = [ - 1, - 101, - 2, - 102, - 3, - 103, - 4, - 104, - 5, - 105, - -1, # marks end of messages from channel 1 - 1000, - -2, # marks end of messages from channel 2 - ] - assert results == expected_results - got_err = False - try: - item = select.unknown_channel - except KeyError: - got_err = True - assert got_err From d46f36f87ed3184aa1d8a69ef9db6d60f0e0a9b8 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 19 Jun 2023 14:27:38 +0200 Subject: [PATCH 13/20] Rename the util._select module to util._selector This is to match the main contents of the module, the `Selector` class. Signed-off-by: Leandro Lucarella --- src/frequenz/channels/util/__init__.py | 2 +- src/frequenz/channels/util/{_select.py => _selector.py} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename src/frequenz/channels/util/{_select.py => _selector.py} (100%) diff --git a/src/frequenz/channels/util/__init__.py b/src/frequenz/channels/util/__init__.py index ca6a21a5..93415723 100644 --- a/src/frequenz/channels/util/__init__.py +++ b/src/frequenz/channels/util/__init__.py @@ -31,7 +31,7 @@ from ._file_watcher import FileWatcher from ._merge import Merge from ._merge_named import MergeNamed -from ._select import ( +from ._selector import ( Selected, SelectError, SelectErrorGroup, diff --git a/src/frequenz/channels/util/_select.py b/src/frequenz/channels/util/_selector.py similarity index 100% rename from src/frequenz/channels/util/_select.py rename to src/frequenz/channels/util/_selector.py From 512837bd2edaffef7f627e764592bcf3278785ed Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 19 Jun 2023 16:55:00 +0200 Subject: [PATCH 14/20] Update release notes Signed-off-by: Leandro Lucarella --- RELEASE_NOTES.md | 84 +++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 80 insertions(+), 4 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 9ca39d45..1dc1205d 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -2,16 +2,92 @@ ## Summary - +The minimum Python supported version was bumped to 3.11 and the `Select` class replaced by the new `Selector`. ## Upgrading * The minimum supported Python version was bumped to 3.11, downstream projects will need to upgrade too to use this version. +* The `Select` class was replaced by a new `Selector` class, with the following improvements: + + * Type-safe: proper type hinting by using the new helper type guard `selected_from()`. + * Fixes potential starvation issues. + * Simplifies the interface by providing values one-by-one. + * Guarantees there are no dangling tasks left behind when used as an async context manager. + + This new class acts as an [async context manager](https://docs.python.org/3/reference/datamodel.html#async-context-managers) and an [async iterator](https://docs.python.org/3.11/library/collections.abc.html#collections.abc.AsyncIterator), and makes sure no dangling tasks are left behind after a select loop is done. + + Example: + ```python + timer1 = Timer.periodic(datetime.timedelta(seconds=1)) + timer2 = Timer.timeout(datetime.timedelta(seconds=0.5)) + + async with Selector(timer1, timer2) as selector: + async for selected in selector: + if selected_from(selected, timer1): + # Beware: `selected.value` might raise an exception, you can always + # check for exceptions with `selected.exception` first or use + # a try-except block. You can also quickly check if the receiver was + # stopped and let any other unexpected exceptions bubble up. + if selected.was_stopped(): + print("timer1 was stopped") + continue + print(f"timer1: now={datetime.datetime.now()} drift={selected.value}") + timer2.stop() + elif selected_from(selected, timer2): + # Explicitly handling of exceptions + match selected.exception: + case ReceiverStoppedError(): + print("timer2 was stopped") + case Exception() as exception: + print(f"timer2: exception={exception}") + case None: + # All good, no exception, we can use `selected.value` safely + print( + f"timer2: now={datetime.datetime.now()} " + f"drift={selected.value}" + ) + case _ as unhanded: + assert_never(unhanded) + else: + # This is not necessary, as select() will check for exhaustiveness, but + # it is good practice to have it in case you forgot to handle a new + # receiver added to `select()` at a later point in time. + assert False + ``` + ## New Features - +* A new `Selector` class was added, please look at the *Upgrading* section for details. + +* A new `Event` utility receiver was added. + + This receiver can be made ready manually. It is mainly useful for testing but can also become handy in scenarios where a simple, on-off signal needs to be sent to a select loop for example. + + Example: + + ```python + import asyncio + from frequenz.channels import Receiver + from frequenz.channels.util import Event, select, selected_from + + other_receiver: Receiver[int] = ... + exit_event = Event() + + async def exit_after_10_seconds() -> None: + asyncio.sleep(10) + exit_event.set() + + asyncio.ensure_future(exit_after_10_seconds()) -## Bug Fixes + async with Selector(exit_event, other_receiver) as selector: + async for selected in selector: + if selected_from(selected, exit_event): + break + if selected_from(selected, other_receiver): + print(selected.value) + else: + assert False, "Unknow receiver selected" + ``` - +* The `Timer` class now has more descriptive `__str__` and `__repr__` methods. From 6785cfc98d203a0365752580b58610a7857bedec Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 30 Jun 2023 15:38:50 +0200 Subject: [PATCH 15/20] Add a new async generator implementation for select() It turns out the *broken* async generators are not completely leaked, it just needs more "time" to be cleaned up. In the example in 9015e7da0f9faeb2bae470d62c040f9ea1839ceb if a few `await asyncio.sleep(0)` are added, then the generator is properly finalized. Even when the destruction is much less deterministic than using a context manager, it should be safe enough to assume that in a long running program the generator will be eventually (and very soon) cleaned up, and the user interface is much simpler and intuitive, so we decided to bring it back. This in a way reverts 9015e7da0f9faeb2bae470d62c040f9ea1839ceb but it also simplifies the code and fixes a few bugs. The tests of the selector are ported to test select() and we make sure all tasks are properly cleaned up after a few yields to the event loop. Signed-off-by: Leandro Lucarella --- src/frequenz/channels/util/__init__.py | 5 + src/frequenz/channels/util/_selector.py | 162 ++++++++- tests/utils/test_select_integration.py | 448 ++++++++++++++++++++++++ 3 files changed, 614 insertions(+), 1 deletion(-) create mode 100644 tests/utils/test_select_integration.py diff --git a/src/frequenz/channels/util/__init__.py b/src/frequenz/channels/util/__init__.py index 93415723..560b8f53 100644 --- a/src/frequenz/channels/util/__init__.py +++ b/src/frequenz/channels/util/__init__.py @@ -23,6 +23,9 @@ * [Timer][frequenz.channels.util.Timer]: A [receiver][frequenz.channels.Receiver] that ticks at certain intervals. +* [select][frequenz.channels.util.select]: Iterate over the values of all + [receivers][frequenz.channels.Receiver] as new values become available. + * [Selector][frequenz.channels.util.Selector]: A tool to iterate over the values of all [receivers][frequenz.channels.Receiver] as new values become available. """ @@ -37,6 +40,7 @@ SelectErrorGroup, Selector, UnhandledSelectedError, + select, selected_from, ) from ._timer import ( @@ -62,5 +66,6 @@ "SkipMissedAndResync", "TriggerAllMissed", "UnhandledSelectedError", + "select", "selected_from", ] diff --git a/src/frequenz/channels/util/_selector.py b/src/frequenz/channels/util/_selector.py index a921a055..5d936c91 100644 --- a/src/frequenz/channels/util/_selector.py +++ b/src/frequenz/channels/util/_selector.py @@ -10,7 +10,7 @@ import asyncio from types import TracebackType -from typing import Any, Generic, Self, TypeGuard, TypeVar, assert_never +from typing import Any, AsyncIterator, Generic, Self, TypeGuard, TypeVar, assert_never from .._base_classes import Receiver from .._exceptions import ReceiverStoppedError @@ -239,6 +239,166 @@ class SelectErrorGroup(BaseExceptionGroup[BaseException], SelectError): # https://github.com/python/mypy/issues/13597 +async def select(*receivers: Receiver[Any]) -> AsyncIterator[Selected[Any]]: + """Iterate over the values of all receivers as they receive new values. + + This function is used to iterate over the values of all receivers as they receive + new values. It is used in conjunction with the + [`Selected`][frequenz.channels.util.Selected] class and the + [`selected_from()`][frequenz.channels.util.selected_from] function to determine + which function to determine which receiver was selected in a select operation. + + An exhaustiveness check is performed at runtime to make sure all selected receivers + are handled in the if-chain, so you should call `selected_from()` with all the + receivers passed to `select()` inside the select loop, even if you plan to ignore + a value, to signal `select()` that you are purposefully ignoring the value. + + Note: + The `select()` function is intended to be used in cases where the set of + receivers is static and known beforehand. If you need to dynamically add/remove + receivers from a select loop, there are a few alternatives. Depending on your + use case, one or the other could work better for you: + + * Use [`Merge`][frequenz.channels.util.Merge] or + [`MergeNamed`][frequenz.channels.util.MergeNamed]: this is useful when you + have and unknown number of receivers of the same type that can be handled as + a group. + * Use tasks to manage each recever individually: this is better if there are no + relationships between the receivers. + * Break the `select()` loop and start a new one with the new set of receivers + (this should be the last resort, as it has some performance implications + because the loop needs to be restarted). + + Example: + ```python + import datetime + from typing import assert_never + + from frequenz.channels import ReceiverStoppedError + from frequenz.channels.util import select, selected_from, Timer + + timer1 = Timer.periodic(datetime.timedelta(seconds=1)) + timer2 = Timer.timeout(datetime.timedelta(seconds=0.5)) + + async for selected in select(timer1, timer2): + if selected_from(selected, timer1): + # Beware: `selected.value` might raise an exception, you can always + # check for exceptions with `selected.exception` first or use + # a try-except block. You can also quickly check if the receiver was + # stopped and let any other unexpected exceptions bubble up. + if selected.was_stopped: + print("timer1 was stopped") + continue + print(f"timer1: now={datetime.datetime.now()} drift={selected.value}") + timer2.stop() + elif selected_from(selected, timer2): + # Explicitly handling of exceptions + match selected.exception: + case ReceiverStoppedError(): + print("timer2 was stopped") + case Exception() as exception: + print(f"timer2: exception={exception}") + case None: + # All good, no exception, we can use `selected.value` safely + print( + f"timer2: now={datetime.datetime.now()} drift={selected.value}" + ) + case _ as unhanded: + assert_never(unhanded) + else: + # This is not necessary, as select() will check for exhaustiveness, but + # it is good practice to have it in case you forgot to handle a new + # receiver added to `select()` at a later point in time. + assert False + ``` + + Args: + *receivers: The receivers to select from. + + Yields: + The currently selected item. + + Raises: + UnhandledSelectedError: If a selected receiver was not handled in the if-chain. + SelectErrorGroup: If there is an error while finishing the select operation and + receivers fail while cleaning up. + SelectError: If there is an error while selecting receivers during normal + operation. For example if a receiver raises an exception in the `ready()` + method. Normal errors while receiving values are not raised, but reported + via the `Selected` instance. + """ + receivers_map: dict[str, Receiver[Any]] = {str(hash(r)): r for r in receivers} + pending: set[asyncio.Task[bool]] = set() + + try: + for name, recv in receivers_map.items(): + pending.add(asyncio.create_task(recv.ready(), name=name)) + + while pending: + done, pending = await asyncio.wait( + pending, return_when=asyncio.FIRST_COMPLETED + ) + + for task in done: + receiver_active: bool = True + name = task.get_name() + recv = receivers_map[name] + if exception := task.exception(): + match exception: + case asyncio.CancelledError(): + # If the receiver was cancelled, then it means we want to + # exit the select loop, so we handle the receiver but we + # don't add it back to the pending list. + receiver_active = False + case _ as exc: + raise SelectError(f"Error while selecting {recv}") from exc + + selected = Selected(recv) + yield selected + if not selected._handled: # pylint: disable=protected-access + raise UnhandledSelectedError(selected) + + receiver_active = task.result() + if not receiver_active: + continue + + # Add back the receiver to the pending list + name = task.get_name() + recv = receivers_map[name] + pending.add(asyncio.create_task(recv.ready(), name=name)) + finally: + await _stop_pending_tasks(pending) + + +async def _stop_pending_tasks(pending: set[asyncio.Task[bool]]) -> None: + """Stop all pending tasks. + + Args: + pending: The pending tasks to stop. + + Raises: + SelectErrorGroup: If the receivers raise any exceptions. + """ + if pending: + for task in pending: + task.cancel() + done, pending = await asyncio.wait(pending) + assert not pending + exceptions: list[BaseException] = [] + for task in done: + if task.cancelled(): + continue + if exception := task.exception(): + exceptions.append(exception) + if exceptions: + # If the select loop is interrupted by a break or exception, then this + # exception will be actually swallowed, as the select() async generator + # will be collected by the asyncio loop. This shouldn't be too bad as + # errors produced by receivers will be re-raised when trying to use them + # again. + raise SelectErrorGroup("Some receivers failed when select()ing", exceptions) + + class Selector: """A tool to iterate over the values of all receivers as new values become available. diff --git a/tests/utils/test_select_integration.py b/tests/utils/test_select_integration.py new file mode 100644 index 00000000..7d2ff997 --- /dev/null +++ b/tests/utils/test_select_integration.py @@ -0,0 +1,448 @@ +# License: MIT +# Copyright © 2023 Frequenz Energy-as-a-Service GmbH + +"""Integration tests for Select function. + +These tests are actually a bit in the middle between unit and integration, because we +are using a fake loop to make the tests faster, but we are still testing more than one +class at a time. +""" + +import asyncio +from collections.abc import AsyncIterator, Iterator +from typing import Any + +import async_solipsism +import pytest + +from frequenz.channels import Receiver, ReceiverStoppedError +from frequenz.channels.util import ( + Event, + Selected, + UnhandledSelectedError, + select, + selected_from, +) + + +@pytest.mark.integration +class TestSelect: + """Tests for the select function.""" + + recv1: Event + recv2: Event + recv3: Event + loop: async_solipsism.EventLoop + + @pytest.fixture(autouse=True) + def event_loop( + self, request: pytest.FixtureRequest + ) -> Iterator[async_solipsism.EventLoop]: + """Replace the loop with one that doesn't interact with the outside world.""" + loop = async_solipsism.EventLoop() + request.cls.loop = loop + yield loop + loop.close() + + @pytest.fixture() + async def start_run_ordered_sequence(self) -> AsyncIterator[asyncio.Task[None]]: + """Start the run_ordered_sequence method and wait for it to finish. + + Yields: + The task running the run_ordered_sequence method. + """ + sequence_task = asyncio.create_task(self.run_ordered_sequence()) + yield sequence_task + await sequence_task + + def setup_method(self) -> None: + """Set up the test.""" + self.recv1 = Event("recv1") + self.recv2 = Event("recv2") + self.recv3 = Event("recv3") + + def assert_received_from( + self, + selected: Selected[Any], + receiver: Receiver[None], + *, + at_time: float, + expected_pending_tasks: int = -2, + ) -> None: + """Assert that the selected event was received from the given receiver. + + It also asserts that: + + * The receiver didn't raise an exception. + * The receiver wasn't stopped. + * The select loop is still running. + * It happened at the given time. + + Args: + selected: The selected event. + receiver: The receiver from which the event was received. + at_time: The time at which the event was received. + expected_pending_tasks: Check that a number of tasks are pending. If the + number is negative, a > check is performed with the absolute value. If + it is 0, no check is performed. + """ + assert selected_from(selected, receiver) + assert selected.value is None + assert selected.exception is None + assert not selected.was_stopped() + if expected_pending_tasks > 0: + assert len(asyncio.all_tasks(self.loop)) == expected_pending_tasks + elif expected_pending_tasks < 0: + assert len(asyncio.all_tasks(self.loop)) > expected_pending_tasks + assert self.loop.time() == at_time + + def assert_receiver_stopped( + self, + selected: Selected[Any], + receiver: Receiver[None], + *, + at_time: float, + expected_pending_tasks: int = -2, + ) -> None: + """Assert that the selected event came from a stopped receiver. + + It also asserts that: + + * The amount of pending tasks is as expected. + * It happened at the given time. + + Args: + selected: The selected event. + receiver: The receiver from which the event was received. + at_time: The time at which the event was received. + expected_pending_tasks: Check that a number of tasks are pending. If the + number is negative, a > check is performed with the absolute value. If + it is 0, no check is performed. + """ + assert selected_from(selected, receiver) + assert selected.was_stopped() + assert isinstance(selected.exception, ReceiverStoppedError) + assert selected.exception.receiver is receiver + if expected_pending_tasks > 0: + assert len(asyncio.all_tasks(self.loop)) == expected_pending_tasks + elif expected_pending_tasks < 0: + assert len(asyncio.all_tasks(self.loop)) > expected_pending_tasks + assert self.loop.time() == at_time + + # We use the loop time (and the sleeps in the run_ordered_sequence method) mainly to + # ensure we are processing the events in the correct order and we are really + # following the sequence of events we expect. + + async def run_ordered_sequence(self) -> None: + """Run the sequence of events to be tested.""" + print("time = 0") + self.recv1.set() + await asyncio.sleep(1) + + print("time = 1") + self.recv2.set() + await asyncio.sleep(1) + + print("time = 2") + self.recv3.set() + await asyncio.sleep(1) + + print("time = 3") + self.recv1.set() + await asyncio.sleep(1) + + print("time = 4") + self.recv1.set() + await asyncio.sleep(1) + + print("time = 5") + self.recv3.set() + await asyncio.sleep(1) + + print("time = 6") + self.recv2.set() + await asyncio.sleep(1) + + print("time = 7") + self.recv1.stop() + await asyncio.sleep(1) + + print("time = 8") + self.recv2.set() + await asyncio.sleep(1) + + print("time = 9") + self.recv3.stop() + await asyncio.sleep(1) + + print("time = 10") + self.recv2.set() + await asyncio.sleep(1) + + print("time = 11") + self.recv2.stop() + + # pylint: disable=redefined-outer-name + async def test_select_receives_in_order( + self, + start_run_ordered_sequence: asyncio.Task[ # pylint: disable=unused-argument + None + ], + ) -> None: + """Test that the select loop receives events in the correct order.""" + select_iter = select(self.recv1, self.recv2, self.recv3) + + selected = await anext(select_iter) + self.assert_received_from(selected, self.recv1, at_time=0) + + selected = await anext(select_iter) + self.assert_received_from(selected, self.recv2, at_time=1) + + selected = await anext(select_iter) + self.assert_received_from(selected, self.recv3, at_time=2) + + selected = await anext(select_iter) + self.assert_received_from(selected, self.recv1, at_time=3) + + selected = await anext(select_iter) + self.assert_received_from(selected, self.recv1, at_time=4) + + selected = await anext(select_iter) + self.assert_received_from(selected, self.recv3, at_time=5) + + selected = await anext(select_iter) + self.assert_received_from(selected, self.recv2, at_time=6) + + selected = await anext(select_iter) + self.assert_receiver_stopped(selected, self.recv1, at_time=7) + + selected = await anext(select_iter) + self.assert_received_from(selected, self.recv2, at_time=8) + + selected = await anext(select_iter) + self.assert_receiver_stopped(selected, self.recv3, at_time=9) + + selected = await anext(select_iter) + self.assert_received_from(selected, self.recv2, at_time=10) + + selected = await anext(select_iter) + self.assert_receiver_stopped( + selected, self.recv2, at_time=11, expected_pending_tasks=1 + ) + + with pytest.raises(StopAsyncIteration): + selected = await anext(select_iter) + + assert len(asyncio.all_tasks()) == 1 # Only the test task should be alive + + async def test_break( + self, + start_run_ordered_sequence: asyncio.Task[ # pylint: disable=unused-argument + None + ], + ) -> None: + """Test that break works.""" + selected: Selected[Any] | None = None + async for selected in select(self.recv1, self.recv2, self.recv3): + if selected_from(selected, self.recv1): + continue + if selected_from(selected, self.recv2): + continue + if selected_from(selected, self.recv3): + break + + assert selected is not None + self.assert_received_from(selected, self.recv3, at_time=2) + + async for selected in select(self.recv1, self.recv2, self.recv3): + if selected_from(selected, self.recv1): + continue + if selected_from(selected, self.recv2): + break + if selected_from(selected, self.recv3): + continue + + assert selected is not None + self.assert_received_from(selected, self.recv2, at_time=6) + + async for selected in select(self.recv1, self.recv2, self.recv3): + if selected_from(selected, self.recv1): + continue + if selected_from(selected, self.recv2): + continue + if selected_from(selected, self.recv3): + break + + assert selected is not None + self.assert_receiver_stopped(selected, self.recv3, at_time=9) + + assert self.recv1.is_stopped + assert self.recv3.is_stopped + + async for selected in select(self.recv2): + if selected_from(selected, self.recv2): + continue + + self.assert_receiver_stopped( + selected, self.recv2, at_time=11, expected_pending_tasks=1 + ) + + assert len(asyncio.all_tasks()) == 1 # Only the test task should be alive + + async def test_missed_select_from( + self, + start_run_ordered_sequence: asyncio.Task[ # pylint: disable=unused-argument + None + ], + ) -> None: + """Test that a missed `select_from` is detected.""" + selected: Selected[Any] | None = None + with pytest.raises(UnhandledSelectedError) as excinfo: + async for selected in select(self.recv1, self.recv2, self.recv3): + if selected_from(selected, self.recv1): + continue + if selected_from(selected, self.recv2): + continue + + assert False, "Should not reach this point" + + assert selected is not None + assert excinfo.value.selected is selected + self.assert_received_from( + selected, self.recv3, at_time=2, expected_pending_tasks=2 + ) + + # The test task and the run_ordered_sequence tasks should still be alive + assert len(asyncio.all_tasks()) == 2 + assert start_run_ordered_sequence in asyncio.all_tasks() + + @pytest.fixture() + async def start_run_multiple_ready(self) -> AsyncIterator[asyncio.Task[None]]: + """Start the run_multiple_ready method and wait for it to finish. + + Yields: + The task running the run_multiple_ready method. + """ + sequence_task = asyncio.create_task(self.run_multiple_ready()) + yield sequence_task + await sequence_task + + async def run_multiple_ready(self) -> None: + """Run a sequence of events with multiple receivers ready.""" + print("time = 0") + self.recv1.set() + self.recv2.set() + self.recv3.set() + await asyncio.sleep(1) + + print("time = 1") + self.recv2.set() + self.recv3.set() + await asyncio.sleep(1) + + print("time = 2") + self.recv1.set() + self.recv3.set() + await asyncio.sleep(1) + + print("time = 3") + self.recv1.set() + self.recv2.set() + await asyncio.sleep(1) + + print("time = 4") + + async def test_multiple_ready( + self, + start_run_multiple_ready: asyncio.Task[None], # pylint: disable=unused-argument + ) -> None: + """Test that multiple ready receviers are handled properly. + + Also test that the loop waits forever if there are no more receivers ready. + """ + received: set[str] = set() + last_time: float = self.loop.time() + try: + async with asyncio.timeout(15): + async for selected in select(self.recv1, self.recv2, self.recv3): + now = self.loop.time() + if now != last_time: # Only check when there was a jump in time + match now: + case 1: + assert received == { + self.recv1.name, + self.recv2.name, + self.recv3.name, + } + case 2: + assert received == { + self.recv2.name, + self.recv3.name, + } + case 3: + assert received == { + self.recv1.name, + self.recv3.name, + } + # case 4 needs to be checked after the timeout, as there + # are no ready receivers after time == 3. + case _: + assert False, "Should not reach this point" + received.clear() + last_time = now + + if selected_from(selected, self.recv1): + received.add(self.recv1.name) + elif selected_from(selected, self.recv2): + received.add(self.recv2.name) + elif selected_from(selected, self.recv3): + received.add(self.recv3.name) + else: + assert False, "Should not reach this point" + except asyncio.TimeoutError: + assert self.loop.time() == 15 + # This happened after time == 3, but the loop never resumes becuase + # there is nothing ready, so we need to check it after the timeout. + assert received == { + self.recv1.name, + self.recv2.name, + } + else: + assert False, "Should have timed out" + + assert len(asyncio.all_tasks()) == 1 # The test task should still be alive + + def test_tasks_are_cleaned_up_with_break(self) -> None: + """Test that the tasks are cleaned up properly. + + In this test we use a real event loop instead of relying what is provided by + pytest to make absolutely sure that the tasks are cleaned up properly with + a real loop. + """ + loop = asyncio.new_event_loop() + + async def run() -> None: + task = loop.create_task(self.run_multiple_ready()) + async for selected in select(self.recv1, self.recv2, self.recv3): + if selected_from(selected, self.recv1): + continue + if selected_from(selected, self.recv2): + continue + if selected_from(selected, self.recv3): + break + + task.cancel() + try: + await task + except asyncio.CancelledError: + pass + + # The loop might take a few "yields" to process all pending tasks and ensure + # the finalized of select() was called + iterations = 0 + while len(asyncio.all_tasks(loop)) > 1 and iterations < 5: + await asyncio.sleep(0) + + assert len(asyncio.all_tasks(loop)) == 1 + + loop.run_until_complete(run()) From ea408122db9082b0acab8f4c36cf24d1ceb6d77e Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 30 Jun 2023 15:39:04 +0200 Subject: [PATCH 16/20] Sort exported symbols Signed-off-by: Leandro Lucarella --- src/frequenz/channels/util/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/frequenz/channels/util/__init__.py b/src/frequenz/channels/util/__init__.py index 560b8f53..767977a4 100644 --- a/src/frequenz/channels/util/__init__.py +++ b/src/frequenz/channels/util/__init__.py @@ -57,13 +57,13 @@ "Merge", "MergeNamed", "MissedTickPolicy", - "Timer", "SelectError", "SelectErrorGroup", "Selected", "Selector", "SkipMissedAndDrift", "SkipMissedAndResync", + "Timer", "TriggerAllMissed", "UnhandledSelectedError", "select", From 80341a58235335310d39047d16f24cf6b85c6200 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 30 Jun 2023 15:45:07 +0200 Subject: [PATCH 17/20] Port all existing uses of Selector to select Signed-off-by: Leandro Lucarella --- .github/ISSUE_TEMPLATE/bug.yml | 2 +- src/frequenz/channels/_anycast.py | 2 +- src/frequenz/channels/_broadcast.py | 2 +- src/frequenz/channels/util/_timer.py | 54 +++++++++++------------ tests/utils/test_integration.py | 66 +++++++++++++--------------- 5 files changed, 60 insertions(+), 66 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug.yml b/.github/ISSUE_TEMPLATE/bug.yml index 0119e0ed..d9b39289 100644 --- a/.github/ISSUE_TEMPLATE/bug.yml +++ b/.github/ISSUE_TEMPLATE/bug.yml @@ -49,7 +49,7 @@ body: - Unit, integration and performance tests (part:tests) - Build script, CI, dependencies, etc. (part:tooling) - Channels, `Broadcast`, `Bidirectional`, etc. (part:channels) - - Selector (part:selector) + - Select (part:select) - Utility receivers, `Merge`, etc. (part:receivers) validations: required: true diff --git a/src/frequenz/channels/_anycast.py b/src/frequenz/channels/_anycast.py index 623da0ac..cbdf3d1e 100644 --- a/src/frequenz/channels/_anycast.py +++ b/src/frequenz/channels/_anycast.py @@ -28,7 +28,7 @@ class Anycast(Generic[T]): thread-safe. When there are multiple channel receivers, they can be awaited - simultaneously using a [Selector][frequenz.channels.util.Selector], + simultaneously using [select][frequenz.channels.util.select], [Merge][frequenz.channels.util.Merge] or [MergeNamed][frequenz.channels.util.MergeNamed]. diff --git a/src/frequenz/channels/_broadcast.py b/src/frequenz/channels/_broadcast.py index 33c51733..33f61a80 100644 --- a/src/frequenz/channels/_broadcast.py +++ b/src/frequenz/channels/_broadcast.py @@ -38,7 +38,7 @@ class Broadcast(Generic[T]): are thread-safe. Because of this, `Broadcast` channels are thread-safe. When there are multiple channel receivers, they can be awaited - simultaneously using a [Selector][frequenz.channels.util.Selector], + simultaneously using [select][frequenz.channels.util.select], [Merge][frequenz.channels.util.Merge] or [MergeNamed][frequenz.channels.util.MergeNamed]. diff --git a/src/frequenz/channels/util/_timer.py b/src/frequenz/channels/util/_timer.py index e87cdf84..02c764d6 100644 --- a/src/frequenz/channels/util/_timer.py +++ b/src/frequenz/channels/util/_timer.py @@ -307,12 +307,12 @@ class Timer(Receiver[timedelta]): print(f"The timer has triggered {drift=}") ``` - But you can also use a [`Selector`][frequenz.channels.util.Selector] to combine + But you can also use a [`select`][frequenz.channels.util.select] to combine it with other receivers, and even start it (semi) manually: ```python import logging - from frequenz.channels.util import Selector, selected_from + from frequenz.channels.util import select, selected_from from frequenz.channels import Broadcast timer = Timer.timeout(timedelta(seconds=1.0), auto_start=False) @@ -322,22 +322,21 @@ class Timer(Receiver[timedelta]): timer = Timer.timeout(timedelta(seconds=1.0), auto_start=False) # Do some other initialization, the timer will start automatically if # a message is awaited (or manually via `reset()`). - async with Selector(battery_data, timer) as selector: - async for selected in selector: - if selected_from(selected, battery_data): - if selected.was_closed(): - logging.warning("battery channel closed") - continue - battery_soc = selected.value - elif selected_from(selected, timer): - # Print some regular battery data - print(f"Battery is charged at {battery_soc}%") + async for selected in select(battery_data, timer): + if selected_from(selected, battery_data): + if selected.was_closed(): + logging.warning("battery channel closed") + continue + battery_soc = selected.value + elif selected_from(selected, timer): + # Print some regular battery data + print(f"Battery is charged at {battery_soc}%") ``` Example: Timeout example ```python import logging - from frequenz.channels.util import Selector, selected_from + from frequenz.channels.util import select, selected_from from frequenz.channels import Broadcast def process_data(data: int): @@ -351,21 +350,20 @@ def do_heavy_processing(data: int): chan2 = Broadcast[int]("input-chan-2") battery_data = chan1.new_receiver() heavy_process = chan2.new_receiver() - async with Selector(battery_data, heavy_process, timer) as selector: - async for selected in selector: - if selected_from(selected, battery_data): - if selected.was_closed(): - logging.warning("battery channel closed") - continue - process_data(selected.value) - timer.reset() - elif selected_from(selected, heavy_process): - if selected.was_closed(): - logging.warning("processing channel closed") - continue - do_heavy_processing(selected.value) - elif selected_from(selected, timer): - logging.warning("No data received in time") + async for selected in select(battery_data, heavy_process, timer): + if selected_from(selected, battery_data): + if selected.was_closed(): + logging.warning("battery channel closed") + continue + process_data(selected.value) + timer.reset() + elif selected_from(selected, heavy_process): + if selected.was_closed(): + logging.warning("processing channel closed") + continue + do_heavy_processing(selected.value) + elif selected_from(selected, timer): + logging.warning("No data received in time") ``` In this case `do_heavy_processing` might take 2 seconds, and we don't diff --git a/tests/utils/test_integration.py b/tests/utils/test_integration.py index bb3dec86..e61cb620 100644 --- a/tests/utils/test_integration.py +++ b/tests/utils/test_integration.py @@ -9,7 +9,7 @@ import pytest -from frequenz.channels.util import FileWatcher, Selector, Timer, selected_from +from frequenz.channels.util import FileWatcher, Timer, select, selected_from @pytest.mark.integration @@ -27,23 +27,20 @@ async def test_file_watcher(tmp_path: pathlib.Path) -> None: file_watcher = FileWatcher(paths=[str(tmp_path)]) timer = Timer.timeout(timedelta(seconds=0.1)) - async with Selector(file_watcher, timer) as selector: - async for selected in selector: - if selected_from(selected, timer): - filename.write_text(f"{selected.value}") - elif selected_from(selected, file_watcher): - event_type = ( - FileWatcher.EventType.CREATE - if number_of_writes == 0 - else FileWatcher.EventType.MODIFY - ) - assert selected.value == FileWatcher.Event( - type=event_type, path=filename - ) - number_of_writes += 1 - # After receiving a write 3 times, unsubscribe from the writes channel - if number_of_writes == expected_number_of_writes: - break + async for selected in select(file_watcher, timer): + if selected_from(selected, timer): + filename.write_text(f"{selected.value}") + elif selected_from(selected, file_watcher): + event_type = ( + FileWatcher.EventType.CREATE + if number_of_writes == 0 + else FileWatcher.EventType.MODIFY + ) + assert selected.value == FileWatcher.Event(type=event_type, path=filename) + number_of_writes += 1 + # After receiving a write 3 times, unsubscribe from the writes channel + if number_of_writes == expected_number_of_writes: + break assert number_of_writes == expected_number_of_writes @@ -85,23 +82,22 @@ async def test_file_watcher_deletes(tmp_path: pathlib.Path) -> None: # W: Write # D: Delete # E: FileWatcher Event - async with Selector(file_watcher, write_timer, deletion_timer) as selector: - async for selected in selector: - if selected_from(selected, write_timer): - if number_of_write >= 2 and number_of_events == 0: - continue - filename.write_text(f"{selected.value}") - number_of_write += 1 - elif selected_from(selected, deletion_timer): - # Avoid removing the file twice - if not pathlib.Path(filename).is_file(): - continue - os.remove(filename) - number_of_deletes += 1 - elif selected_from(selected, file_watcher): - number_of_events += 1 - if number_of_events >= 2: - break + async for selected in select(file_watcher, write_timer, deletion_timer): + if selected_from(selected, write_timer): + if number_of_write >= 2 and number_of_events == 0: + continue + filename.write_text(f"{selected.value}") + number_of_write += 1 + elif selected_from(selected, deletion_timer): + # Avoid removing the file twice + if not pathlib.Path(filename).is_file(): + continue + os.remove(filename) + number_of_deletes += 1 + elif selected_from(selected, file_watcher): + number_of_events += 1 + if number_of_events >= 2: + break assert number_of_deletes == 2 # Can be more because the watcher could take some time to trigger From da8189326169977498049fad11eaf21ddce1ee4f Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 30 Jun 2023 15:53:43 +0200 Subject: [PATCH 18/20] Remove the Selector class and tests This was replaced by the `select()` function. Signed-off-by: Leandro Lucarella --- src/frequenz/channels/util/__init__.py | 5 - src/frequenz/channels/util/_selector.py | 394 +----------------- tests/utils/test_selector.py | 2 +- tests/utils/test_selector_integration.py | 491 ----------------------- 4 files changed, 16 insertions(+), 876 deletions(-) delete mode 100644 tests/utils/test_selector_integration.py diff --git a/src/frequenz/channels/util/__init__.py b/src/frequenz/channels/util/__init__.py index 767977a4..c26ccdaa 100644 --- a/src/frequenz/channels/util/__init__.py +++ b/src/frequenz/channels/util/__init__.py @@ -25,9 +25,6 @@ * [select][frequenz.channels.util.select]: Iterate over the values of all [receivers][frequenz.channels.Receiver] as new values become available. - -* [Selector][frequenz.channels.util.Selector]: A tool to iterate over the values of all - [receivers][frequenz.channels.Receiver] as new values become available. """ from ._event import Event @@ -38,7 +35,6 @@ Selected, SelectError, SelectErrorGroup, - Selector, UnhandledSelectedError, select, selected_from, @@ -60,7 +56,6 @@ "SelectError", "SelectErrorGroup", "Selected", - "Selector", "SkipMissedAndDrift", "SkipMissedAndResync", "Timer", diff --git a/src/frequenz/channels/util/_selector.py b/src/frequenz/channels/util/_selector.py index 5d936c91..9978fea7 100644 --- a/src/frequenz/channels/util/_selector.py +++ b/src/frequenz/channels/util/_selector.py @@ -9,8 +9,7 @@ """ import asyncio -from types import TracebackType -from typing import Any, AsyncIterator, Generic, Self, TypeGuard, TypeVar, assert_never +from typing import Any, AsyncIterator, Generic, TypeGuard, TypeVar from .._base_classes import Receiver from .._exceptions import ReceiverStoppedError @@ -19,7 +18,7 @@ class Selected(Generic[_T]): - """A result of a [`Selector`][frequenz.channels.util.Selector] loop iteration. + """A result of a [`select`][frequenz.channels.util.select] iteration. The selected receiver is consumed immediately and the received value is stored in the instance, unless there was an exception while receiving the value, in which case @@ -29,7 +28,7 @@ class Selected(Generic[_T]): [`selected_from()`][frequenz.channels.util.selected_from] function to determine which receiver was selected. - Please see [`Selector`][frequenz.channels.util.Selector] for an example. + Please see [`select`][frequenz.channels.util.select] for an example. """ class _EmptyResult: @@ -142,19 +141,19 @@ def __repr__(self) -> str: def selected_from( selected: Selected[Any], receiver: Receiver[_T] ) -> TypeGuard[Selected[_T]]: - """Check if the given receiver was selected by a [`Selector`][frequenz.channels.util.Selector]. + """Check if the given receiver was selected by [`select`][frequenz.channels.util.select]. This function is used in conjunction with the [`Selected`][frequenz.channels.util.Selected] class to determine which receiver was - selected in a `Selector` loop iteration. + selected in `select()` iteration. It also works as a [type guard][typing.TypeGuard] to narrow the type of the `Selected` instance to the type of the receiver. - Please see [`Selector`][frequenz.channels.util.Selector] for an example. + Please see [`select`][frequenz.channels.util.select] for an example. Args: - selected: The result of a `Selector` loop iteration. + selected: The result of a `select()` iteration. receiver: The receiver to check if it was the source of a select operation. Returns: @@ -166,20 +165,20 @@ def selected_from( class SelectError(BaseException): - """A base exception for [`Selector`][frequenz.channels.util.Selector]. + """A base exception for [`select`][frequenz.channels.util.select]. - This exception is raised when a `Selector` loop iteration fails. It is raised as + This exception is raised when a `select()` iteration fails. It is raised as a single exception when one receiver fails during normal operation (while calling `ready()` for example). It is raised as a group exception - ([`SelectErrorGroup`][frequenz.channels.util.SelectErrorGroup]) when a `Selector` is - [`stop()`][frequenz.channels.util.Selector.stop]ed. + ([`SelectErrorGroup`][frequenz.channels.util.SelectErrorGroup]) when a `select` loop + is cleaning up after it's done. """ class UnhandledSelectedError(SelectError, Generic[_T]): - """A receiver was not handled in a [`Selector`][frequenz.channels.util.Selector] loop. + """A receiver was not handled in a [`select()`][frequenz.channels.util.select] loop. - This exception is raised when a `Selector` loop finishes without a call to + This exception is raised when a `select()` iteration finishes without a call to [`selected_from()`][frequenz.channels.util.selected_from] for the selected receiver. """ @@ -195,9 +194,9 @@ def __init__(self, selected: Selected[_T]) -> None: class SelectErrorGroup(BaseExceptionGroup[BaseException], SelectError): - """An exception group for [`Selector`][frequenz.channels.util.Selector]. + """An exception group for [`select()`][frequenz.channels.util.select] operation. - This exception group is raised when a [`Selector.stop()`] fails while cleaning up + This exception group is raised when a [`select()`] loops fails while cleaning up runing tasts to check for ready receivers. """ @@ -397,366 +396,3 @@ async def _stop_pending_tasks(pending: set[asyncio.Task[bool]]) -> None: # errors produced by receivers will be re-raised when trying to use them # again. raise SelectErrorGroup("Some receivers failed when select()ing", exceptions) - - -class Selector: - """A tool to iterate over the values of all receivers as new values become available. - - This tool is used to iterate over the values of all receivers as they receive new - values. It is used in conjunction with the - [`Selected`][frequenz.channels.util.Selected] class and the - [`selected_from()`][frequenz.channels.util.selected_from] [type - guard][typing.TypeGuard] to determine which receiver was selected in a `Selector` - loop iteration. - - An exhaustiveness check is performed at runtime to make sure all selected receivers - are handled in the loop if-chain. You must call `selected_from()` with all the - receivers passed to the `Selector` inside the selection loop, even if you plan to - ignore a value. This is to signal the `Selector` that you are purposefully ignoring - the value and the exhaustiveness check doesn't fail. - - A `Selector` will create one task per receiver to check if it is ready to receive a - new value. To make sure tasks are properly destroyed after a selection loop, the - `Selector` class is an [async context - manager](https://docs.python.org/3/reference/datamodel.html#async-context-managers). - - When instantiated, a `Selector` [is - stopped][frequenz.channels.util.Selector.is_stopped], and task are not started until - [`start()`][frequenz.channels.util.Selector.start] is called or the async context is - entered. When the async context is exited, the `Selector` is automatically - [`stop()`][frequenz.channels.util.Selector.stop]ed. Because of this, usually you - should not call `start()` or `stop()` manually. - - Note: - A `Selector` is intended to be used in cases where the set of receivers is - static and known beforehand. If you need to dynamically add/remove receivers - from a selector, there are a few alternatives. Depending on your use case, - one or the other could work better for you: - - * Use [`Merge`][frequenz.channels.util.Merge] or - [`MergeNamed`][frequenz.channels.util.MergeNamed]: this is useful when you - have and unknown number of receivers of the same type that can be handled as - a group. - * Use tasks to manage each recever individually: this is better if there are no - relationships between the receivers. - * Break the `Selector` loop and create new instance with the new set of - receivers. This should be the last resort, as it has some performance - implications because the tasks need to be restarted. - - Example: - ```python - import datetime - from typing import assert_never - - from frequenz.channels import ReceiverStoppedError - from frequenz.channels.util import Selector, selected_from, Timer - - timer1 = Timer.periodic(datetime.timedelta(seconds=1)) - timer2 = Timer.timeout(datetime.timedelta(seconds=0.5)) - - async with Selector(timer1, timer2) as selector: - async for selected in selector: - if selected_from(selected, timer1): - # Beware: `selected.value` might raise an exception, you can always - # check for exceptions with `selected.exception` first or use - # a try-except block. You can also quickly check if the receiver was - # stopped and let any other unexpected exceptions bubble up. - if selected.was_stopped(): - print("timer1 was stopped") - continue - print(f"timer1: now={datetime.datetime.now()} drift={selected.value}") - timer2.stop() - elif selected_from(selected, timer2): - # Explicitly handling of exceptions - match selected.exception: - case ReceiverStoppedError(): - print("timer2 was stopped") - case Exception() as exception: - print(f"timer2: exception={exception}") - case None: - # All good, no exception, we can use `selected.value` safely - print( - f"timer2: now={datetime.datetime.now()} " - f"drift={selected.value}" - ) - case _ as unhanded: - assert_never(unhanded) - else: - # This is not necessary, as select() will check for exhaustiveness, but - # it is good practice to have it in case you forgot to handle a new - # receiver added to `select()` at a later point in time. - assert False - ``` - """ - - def __init__(self, *receivers: Receiver[Any]) -> None: - """Create a new instance. - - Creating a new instance of `Selector` does not start it, so no tasks will be - created until [`start()`][frequenz.channels.util.Selector.start] is called. - When a `Selector` is used as a context manager (`async with`) it will - automatically start, so in general there is no need to worry about calling - `start()` manually. - - Args: - *receivers: The receivers to select from. - """ - self._receivers_map: dict[str, Receiver[Any]] = { - str(hash(r)): r for r in receivers - } - """The map of receiver names to receivers. - - This is used to map tasks names with receivers, so that we can easily find the - receiver that was selected when a task completes. - """ - - self._pending_tasks: set[asyncio.Task[bool]] = set() - """The set of pending tasks. - - These tasks are used to wait for the receivers to be ready to receive a new - value. - - If there are no more pending and done tasks, the selector is considered to be - done/stopped. - """ - - self._done_tasks: set[asyncio.Task[bool]] = set() - """The set of done tasks. - - These are tasks that are already done, i.e. the receivers that have a ready - value to be consumed, but that haven't been selected yet. - - If there are no more pending and done tasks, the selector is considered to be - done/stopped. - """ - - self._currently_selected: Selected[Any] | None = None - """The currently selected receiver. - - This is only saved to check for exhaustiveness. - """ - - @property - def is_stopped(self) -> bool: - """Whether this selector is currently stopped. - - Returns: - Whether this selector is currently stopped. - """ - return not self._pending_tasks and not self._done_tasks - - def start(self) -> None: - """Start this selector. - - This will create a task for each receiver passed to the constructor, and - schedule them to run. If this selector is already running, this method does - nothing. - """ - if not self.is_stopped: - return - for name, recv in self._receivers_map.items(): - self._pending_tasks.add(asyncio.create_task(recv.ready(), name=name)) - - def cancel(self) -> None: - """Cancel this selector. - - This will cancel all pending tasks, but won't wait for them to complete. If - you want to wait for them to complete, you can use - [`stop()`][frequenz.channels.util.Selector.stop] instead. - - This can be used to asynchronously stop this selector. - """ - for task in self._pending_tasks: - task.cancel() - - async def stop(self) -> None: - """Stop this selector. - - This will cancel all pending tasks, and wait for them to complete. If you - don't want to wait for them to complete, you can use - [`cancel()`][frequenz.channels.util.Selector.cancel] instead. - - Raises: - SelectErrorGroup: If there is an error in any of the pending receivers. - """ - self.cancel() - if exceptions := await self._wait_for_pending_tasks(): - raise SelectErrorGroup("Some receivers failed when select()ing", exceptions) - - async def __aenter__(self) -> Self: - """Start this selector if it is not already running. - - Returns: - This selector. - - """ - self.start() - return self - - async def __aexit__( - self, - exc_type: type[BaseException] | None, - exc_val: BaseException | None, - exc_tb: TracebackType | None, - ) -> None: - """Stop this selector. - - Args: - exc_type: The type of the exception raised, if any. - exc_val: The exception raised, if any. - exc_tb: The traceback of the exception raised, if any. - """ - await self.stop() - - def __del__(self) -> None: - """Delete this `Selector`. - - Cancel all the pending tasks. - """ - self.cancel() - - def __aiter__(self) -> Self: - """Return self as an async iterator over the selected values. - - Returns: - This selector. - """ - return self - - async def __anext__(self) -> Selected[Any]: - """Iterate over the values of all receivers as they receive new values. - - Returns: - The currently selected item. - - Raises: - UnhandledSelectedError: If the previously selected receiver was not handled - (`select_from` was not called with the selected receiver) in the - if-chain. - SelectError: If there is an internal error in this selector or a receiver - raises an exception while waiting to be ready. Errors while consuming - from the receiver are not raised, but reported via the `Selected` - instance. - StopAsyncIteration: If the `Selector` was not - [`start()`][frequenz.channels.util.Selector.start]ed, all the receivers - were stopped, or [`cancel()`][frequenz.channels.util.Selector.cancel] - was called. - """ - # Check for exhaustiveness - selected = self._currently_selected - self._currently_selected = None - if selected and not selected._handled: # pylint: disable=protected-access - raise UnhandledSelectedError(selected) - - # If we already have some receivers that are done, we can select from them - # without waiting again, but tasks could be done because they were cancelled, - # so we need to check if there is actually a selected receiver. - if self._done_tasks: - if selected := self._select_next(): - return selected - - # If there are no more receivers that are done and no pending receivers either, - # we are done. - # From now on, we need to wait for some pending receivers to be ready. - while self._pending_tasks: - # If there are pending receivers, we wait for some to be ready. - self._done_tasks, self._pending_tasks = await asyncio.wait( - self._pending_tasks, - return_when=asyncio.FIRST_COMPLETED, - ) - - # At this point, **something** should be done, but tasks could be done - # because they were cancelled, so we need to check if there is actually - # a selected receiver. - assert self._done_tasks - if selected := self._select_next(): - return selected - - # If we reached this point, there are no more pending tasks and all done tasks - # were cancelled, so we are done with the loop - assert not self._done_tasks - raise StopAsyncIteration() - - def _select_next(self) -> Selected[Any] | None: - """Select the next receiver that is ready. - - This will select the next receiver that is ready from the list of done tasks, - and return it as a `Selected` instance. - - If there is no receiver that is ready (for example because all done tasks are - from receivers that were stopped or because the tasks were cancelled), this - will return `None`. - - Returns: - The selected receiver, or `None` if there is no selected receiver. - - Raises: - SelectError: If there is an internal error in this selector or a receiver - raises an exception while waiting to be ready. - """ - while self._done_tasks: - done_task = self._done_tasks.pop() - name = done_task.get_name() - recv = self._receivers_map[name] - # If the task was cancelled, we just skip it and don't add if back to the - # pending list because we are being cancelled. - if done_task.cancelled(): - print(f"Cancelled {recv}") - continue - # If there is any other exception (unexpected, as ready() should not - # raise), we re-raise it as a SelectError. - if exception := done_task.exception(): - raise SelectError(f"Error while selecting {recv}") from exception - - self._currently_selected = Selected(recv) - - receiver_active = done_task.result() # False - receiver_stopped = self._currently_selected.was_stopped() # True - if receiver_active and not receiver_stopped: - # Add back the receiver to the pending list if it is still active. - name = done_task.get_name() - recv = self._receivers_map[name] - self._pending_tasks.add(asyncio.create_task(recv.ready(), name=name)) - - return self._currently_selected - return None - - async def _wait_for_pending_tasks(self) -> list[BaseException]: - """Wait for all pending tasks to be done. - - If the pending tasks were cancelled or the underlying receiver was stopped, then - these exceptions will be ignored. Other exceptions will be returned. - - All pending and done tasks will be cleared. - - Returns: - The list of exceptions raised by the pending tasks. - """ - if not self._pending_tasks: - return [] - - self._done_tasks, self._pending_tasks = await asyncio.wait(self._pending_tasks) - assert not self._pending_tasks - exceptions: list[BaseException] = [] - while self._done_tasks: - task = self._done_tasks.pop() - if task.cancelled(): - continue - # The assignment is a workaround for a mypy bug not doing - # proper exhaustiveness checking otherwise: - # https://github.com/python/mypy/issues/12998 - exception = task.exception() - match exception: - # We ignore ReceiverStoppedError too because they will be re-raised if - # the user tries to receive from the receiver again, so there is no - # information loss, and knowing that a receiver stopped when stopping - # the select loop doesn't provide a lot of info and it might even be - # confusing as we swallow ReceiverStoppedError during normal operation - # too. - case None | ReceiverStoppedError(): - pass - case BaseException(): - exceptions.append(exception) - case _ as unhandled: - assert_never(unhandled) - return exceptions diff --git a/tests/utils/test_selector.py b/tests/utils/test_selector.py index efd71ad8..a9a46921 100644 --- a/tests/utils/test_selector.py +++ b/tests/utils/test_selector.py @@ -1,7 +1,7 @@ # License: MIT # Copyright © 2023 Frequenz Energy-as-a-Service GmbH -"""Tests for the Selector implementation.""" +"""Tests for the select implementation.""" from unittest import mock diff --git a/tests/utils/test_selector_integration.py b/tests/utils/test_selector_integration.py deleted file mode 100644 index f604755a..00000000 --- a/tests/utils/test_selector_integration.py +++ /dev/null @@ -1,491 +0,0 @@ -# License: MIT -# Copyright © 2023 Frequenz Energy-as-a-Service GmbH - -"""Integration tests for Selector class. - -These tests are actually a bit in the middle between unit and integration, because we -are using a fake loop to make the tests faster, but we are still testing more than one -class at a time. -""" - -import asyncio -from collections.abc import AsyncIterator, Iterator -from typing import Any - -import async_solipsism -import pytest - -from frequenz.channels import Receiver, ReceiverStoppedError -from frequenz.channels.util import ( - Event, - Selected, - Selector, - UnhandledSelectedError, - selected_from, -) - - -@pytest.mark.integration -class TestSelector: - """Tests for the Selector class.""" - - recv1: Event - recv2: Event - recv3: Event - selector: Selector - loop: async_solipsism.EventLoop - - @pytest.fixture(autouse=True) - def event_loop( - self, request: pytest.FixtureRequest - ) -> Iterator[async_solipsism.EventLoop]: - """Replace the loop with one that doesn't interact with the outside world.""" - loop = async_solipsism.EventLoop() - request.cls.loop = loop - yield loop - loop.close() - - @pytest.fixture() - async def start_run_ordered_sequence(self) -> AsyncIterator[asyncio.Task[None]]: - """Start the run_ordered_sequence method and wait for it to finish. - - Yields: - The task running the run_ordered_sequence method. - """ - sequence_task = asyncio.create_task(self.run_ordered_sequence()) - yield sequence_task - await sequence_task - - def setup_method(self) -> None: - """Set up the test.""" - self.recv1 = Event("recv1") - self.recv2 = Event("recv2") - self.recv3 = Event("recv3") - self.selector = Selector(self.recv1, self.recv2, self.recv3) - - def assert_received_from( - self, - selected: Selected[Any], - receiver: Receiver[None], - *, - at_time: float, - selector_stopped: bool = False, - ) -> None: - """Assert that the selected event was received from the given receiver. - - It also asserts that: - - * The receiver didn't raise an exception. - * The receiver wasn't stopped. - * The selector is still running. - * It happened at the given time. - - Args: - selected: The selected event. - receiver: The receiver from which the event was received. - at_time: The time at which the event was received. - selector_stopped: Check if the selector is stopped or running. - """ - assert selected_from(selected, receiver) - assert selected.value is None - assert selected.exception is None - assert not selected.was_stopped() - assert self.selector.is_stopped == selector_stopped - assert self.loop.time() == at_time - - def assert_receiver_stopped( - self, - selected: Selected[Any], - receiver: Receiver[None], - *, - at_time: float, - selector_stopped: bool = False, - ) -> None: - """Assert that the selected event came from a stopped receiver. - - It also asserts that: - - * The selector is still running (if `selector_stopped` is `False`). - * The selector is stopped (if `selector_stopped` is `True`). - * It happened at the given time. - - Args: - selected: The selected event. - receiver: The receiver from which the event was received. - at_time: The time at which the event was received. - selector_stopped: Check if the selector is stopped or running. - """ - assert selected_from(selected, receiver) - assert selected.was_stopped() - assert isinstance(selected.exception, ReceiverStoppedError) - assert selected.exception.receiver is receiver - assert self.selector.is_stopped == selector_stopped - assert self.loop.time() == at_time - - # We use the loop time (and the sleeps in the run_ordered_sequence method) mainly to - # ensure we are processing the events in the correct order and we are really - # following the sequence of events we expect. - - async def run_ordered_sequence(self) -> None: - """Run the sequence of events to be tested.""" - print("time = 0") - self.recv1.set() - await asyncio.sleep(1) - - print("time = 1") - self.recv2.set() - await asyncio.sleep(1) - - print("time = 2") - self.recv3.set() - await asyncio.sleep(1) - - print("time = 3") - self.recv1.set() - await asyncio.sleep(1) - - print("time = 4") - self.recv1.set() - await asyncio.sleep(1) - - print("time = 5") - self.recv3.set() - await asyncio.sleep(1) - - print("time = 6") - self.recv2.set() - await asyncio.sleep(1) - - print("time = 7") - self.recv1.stop() - await asyncio.sleep(1) - - print("time = 8") - self.recv2.set() - await asyncio.sleep(1) - - print("time = 9") - self.recv3.stop() - await asyncio.sleep(1) - - print("time = 10") - self.recv2.set() - await asyncio.sleep(1) - - print("time = 11") - self.recv2.stop() - - # pylint: disable=redefined-outer-name - async def test_selector_receives_in_order( - self, - start_run_ordered_sequence: asyncio.Task[ # pylint: disable=unused-argument - None - ], - ) -> None: - """Test that the selector receives events in the correct order.""" - assert self.selector.is_stopped - - async with self.selector: - selector_iter = aiter(self.selector) - - selected = await anext(selector_iter) - self.assert_received_from(selected, self.recv1, at_time=0) - - selected = await anext(selector_iter) - self.assert_received_from(selected, self.recv2, at_time=1) - - selected = await anext(selector_iter) - self.assert_received_from(selected, self.recv3, at_time=2) - - selected = await anext(selector_iter) - self.assert_received_from(selected, self.recv1, at_time=3) - - selected = await anext(selector_iter) - self.assert_received_from(selected, self.recv1, at_time=4) - - selected = await anext(selector_iter) - self.assert_received_from(selected, self.recv3, at_time=5) - - selected = await anext(selector_iter) - self.assert_received_from(selected, self.recv2, at_time=6) - - selected = await anext(selector_iter) - self.assert_receiver_stopped(selected, self.recv1, at_time=7) - - selected = await anext(selector_iter) - self.assert_received_from(selected, self.recv2, at_time=8) - - selected = await anext(selector_iter) - self.assert_receiver_stopped(selected, self.recv3, at_time=9) - - selected = await anext(selector_iter) - self.assert_received_from(selected, self.recv2, at_time=10) - - selected = await anext(selector_iter) - self.assert_receiver_stopped( - selected, self.recv2, at_time=11, selector_stopped=True - ) - - with pytest.raises(StopAsyncIteration): - selected = await anext(selector_iter) - assert self.selector.is_stopped - - assert len(asyncio.all_tasks()) == 1 # Only the test task should be alive - assert self.selector.is_stopped - - async def test_break( - self, - start_run_ordered_sequence: asyncio.Task[ # pylint: disable=unused-argument - None - ], - ) -> None: - """Test that break works.""" - assert self.selector.is_stopped - - async with self.selector: - selected: Selected[Any] | None = None - async for selected in self.selector: - if selected_from(selected, self.recv1): - continue - if selected_from(selected, self.recv2): - continue - if selected_from(selected, self.recv3): - break - - assert selected is not None - self.assert_received_from(selected, self.recv3, at_time=2) - - async for selected in self.selector: - if selected_from(selected, self.recv1): - continue - if selected_from(selected, self.recv2): - break - if selected_from(selected, self.recv3): - continue - - assert selected is not None - self.assert_received_from(selected, self.recv2, at_time=6) - - async for selected in self.selector: - if selected_from(selected, self.recv1): - continue - if selected_from(selected, self.recv2): - continue - if selected_from(selected, self.recv3): - break - - assert selected is not None - self.assert_receiver_stopped(selected, self.recv3, at_time=9) - - async for selected in self.selector: - if selected_from(selected, self.recv1): - break - if selected_from(selected, self.recv2): - continue - if selected_from(selected, self.recv3): - break - - self.assert_receiver_stopped( - selected, self.recv2, at_time=11, selector_stopped=True - ) - - assert len(asyncio.all_tasks()) == 1 # Only the test task should be alive - assert self.selector.is_stopped - - async def test_missed_select_from( - self, - start_run_ordered_sequence: asyncio.Task[ # pylint: disable=unused-argument - None - ], - ) -> None: - """Test that a missed `select_from` is detected.""" - assert self.selector.is_stopped - - selected: Selected[Any] | None = None - with pytest.raises(UnhandledSelectedError) as excinfo: - async with self.selector: - async for selected in self.selector: - if selected_from(selected, self.recv1): - continue - if selected_from(selected, self.recv2): - continue - - assert False, "Should not reach this point" - - assert selected is not None - assert excinfo.value.selected is selected - self.assert_received_from( - selected, self.recv3, at_time=2, selector_stopped=True - ) - - # The test task and the run_ordered_sequence tasks should still be alive - assert len(asyncio.all_tasks()) == 2 - assert start_run_ordered_sequence in asyncio.all_tasks() - assert self.selector.is_stopped - - async def test_cancel( - self, - start_run_ordered_sequence: asyncio.Task[ # pylint: disable=unused-argument - None - ], - ) -> None: - """Test that cancel works.""" - assert self.selector.is_stopped - - received: list[str] = [] - async with self.selector: - async for selected in self.selector: - if self.loop.time() == 5: - self.selector.cancel() - if selected_from(selected, self.recv1): - received.append("recv1") - continue - if selected_from(selected, self.recv2): - received.append("recv2") - continue - if selected_from(selected, self.recv3): - received.append("recv3") - continue - - assert received == ["recv1", "recv2", "recv3", "recv1", "recv1", "recv3"] - assert self.selector.is_stopped - - # The test task and the run_ordered_sequence tasks should still be alive - assert len(asyncio.all_tasks()) == 2 - assert start_run_ordered_sequence in asyncio.all_tasks() - assert self.selector.is_stopped - - async def test_manual_start_stop( - self, - start_run_ordered_sequence: asyncio.Task[ # pylint: disable=unused-argument - None - ], - ) -> None: - """Test that cancel works.""" - assert self.selector.is_stopped - - received: list[str] = [] - - self.selector.start() - assert not self.selector.is_stopped - - async for selected in self.selector: - if self.loop.time() == 5: - await self.selector.stop() - if selected_from(selected, self.recv1): - received.append("recv1") - continue - if selected_from(selected, self.recv2): - received.append("recv2") - continue - if selected_from(selected, self.recv3): - received.append("recv3") - continue - - assert received == ["recv1", "recv2", "recv3", "recv1", "recv1", "recv3"] - # The test task and the run_ordered_sequence tasks should still be alive - assert len(asyncio.all_tasks()) == 2 - assert start_run_ordered_sequence in asyncio.all_tasks() - assert self.selector.is_stopped - - @pytest.fixture() - async def start_run_multiple_ready(self) -> AsyncIterator[asyncio.Task[None]]: - """Start the run_multiple_ready method and wait for it to finish. - - Yields: - The task running the run_multiple_ready method. - """ - sequence_task = asyncio.create_task(self.run_multiple_ready()) - yield sequence_task - await sequence_task - - async def run_multiple_ready(self) -> None: - """Run a sequence of events with multiple receivers ready.""" - print("time = 0") - self.recv1.set() - self.recv2.set() - self.recv3.set() - await asyncio.sleep(1) - - print("time = 1") - self.recv2.set() - self.recv3.set() - await asyncio.sleep(1) - - print("time = 2") - self.recv1.set() - self.recv3.set() - await asyncio.sleep(1) - - print("time = 3") - self.recv1.set() - self.recv2.set() - await asyncio.sleep(1) - - print("time = 4") - - async def test_multiple_ready( - self, - start_run_multiple_ready: asyncio.Task[None], # pylint: disable=unused-argument - ) -> None: - """Test that multiple ready receviers are handled properly. - - Also test that the loop waits forever if there are no more receivers ready. - """ - assert self.selector.is_stopped - - received: set[str] = set() - last_time: float = self.loop.time() - async with self.selector: - try: - async with asyncio.timeout(15): - async for selected in self.selector: - now = self.loop.time() - if now != last_time: # Only check when there was a jump in time - match now: - case 1: - assert received == { - self.recv1.name, - self.recv2.name, - self.recv3.name, - } - case 2: - assert received == { - self.recv2.name, - self.recv3.name, - } - case 3: - assert received == { - self.recv1.name, - self.recv3.name, - } - # case 4 needs to be checked after the timeout, as there - # are no ready receivers after time == 3. - case _: - assert False, "Should not reach this point" - received.clear() - last_time = now - - if selected_from(selected, self.recv1): - received.add(self.recv1.name) - elif selected_from(selected, self.recv2): - received.add(self.recv2.name) - elif selected_from(selected, self.recv3): - received.add(self.recv3.name) - else: - assert False, "Should not reach this point" - except asyncio.TimeoutError: - assert self.loop.time() == 15 - # This happened after time == 3, but the loop never resumes becuase - # there is nothing ready, so we need to check it after the timeout. - assert received == { - self.recv1.name, - self.recv2.name, - } - # The selector is still waiting for receivers to be ready. - assert not self.selector.is_stopped - else: - assert False, "Should have timed out" - - assert len(asyncio.all_tasks()) == 1 # The test task should still be alive - assert self.selector.is_stopped From 25be2219fefd032337bcdb1aa5a19e518ae2c80f Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 30 Jun 2023 15:55:28 +0200 Subject: [PATCH 19/20] Renames files from `selector` to `select` Since the `Selector` class is gone, it makes sense to name files as `select` again. Signed-off-by: Leandro Lucarella --- src/frequenz/channels/util/__init__.py | 2 +- src/frequenz/channels/util/{_selector.py => _select.py} | 0 tests/utils/{test_selector.py => test_select.py} | 0 3 files changed, 1 insertion(+), 1 deletion(-) rename src/frequenz/channels/util/{_selector.py => _select.py} (100%) rename tests/utils/{test_selector.py => test_select.py} (100%) diff --git a/src/frequenz/channels/util/__init__.py b/src/frequenz/channels/util/__init__.py index c26ccdaa..515e1ac2 100644 --- a/src/frequenz/channels/util/__init__.py +++ b/src/frequenz/channels/util/__init__.py @@ -31,7 +31,7 @@ from ._file_watcher import FileWatcher from ._merge import Merge from ._merge_named import MergeNamed -from ._selector import ( +from ._select import ( Selected, SelectError, SelectErrorGroup, diff --git a/src/frequenz/channels/util/_selector.py b/src/frequenz/channels/util/_select.py similarity index 100% rename from src/frequenz/channels/util/_selector.py rename to src/frequenz/channels/util/_select.py diff --git a/tests/utils/test_selector.py b/tests/utils/test_select.py similarity index 100% rename from tests/utils/test_selector.py rename to tests/utils/test_select.py From e776d1d656affeb392f20e3d7765bec7fa57d0dd Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 30 Jun 2023 15:58:35 +0200 Subject: [PATCH 20/20] Update release notes Signed-off-by: Leandro Lucarella --- RELEASE_NOTES.md | 86 +++++++++++++++++++++++------------------------- 1 file changed, 42 insertions(+), 44 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 1dc1205d..11da324e 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -2,63 +2,62 @@ ## Summary -The minimum Python supported version was bumped to 3.11 and the `Select` class replaced by the new `Selector`. +The minimum Python supported version was bumped to 3.11 and the `Select` class replaced by the new `select()` function. ## Upgrading * The minimum supported Python version was bumped to 3.11, downstream projects will need to upgrade too to use this version. -* The `Select` class was replaced by a new `Selector` class, with the following improvements: +* The `Select` class was replaced by a new `select()` function, with the following improvements: * Type-safe: proper type hinting by using the new helper type guard `selected_from()`. * Fixes potential starvation issues. * Simplifies the interface by providing values one-by-one. * Guarantees there are no dangling tasks left behind when used as an async context manager. - This new class acts as an [async context manager](https://docs.python.org/3/reference/datamodel.html#async-context-managers) and an [async iterator](https://docs.python.org/3.11/library/collections.abc.html#collections.abc.AsyncIterator), and makes sure no dangling tasks are left behind after a select loop is done. + This new function is an [async iterator](https://docs.python.org/3.11/library/collections.abc.html#collections.abc.AsyncIterator), and makes sure no dangling tasks are left behind after a select loop is done. Example: ```python timer1 = Timer.periodic(datetime.timedelta(seconds=1)) timer2 = Timer.timeout(datetime.timedelta(seconds=0.5)) - async with Selector(timer1, timer2) as selector: - async for selected in selector: - if selected_from(selected, timer1): - # Beware: `selected.value` might raise an exception, you can always - # check for exceptions with `selected.exception` first or use - # a try-except block. You can also quickly check if the receiver was - # stopped and let any other unexpected exceptions bubble up. - if selected.was_stopped(): - print("timer1 was stopped") - continue - print(f"timer1: now={datetime.datetime.now()} drift={selected.value}") - timer2.stop() - elif selected_from(selected, timer2): - # Explicitly handling of exceptions - match selected.exception: - case ReceiverStoppedError(): - print("timer2 was stopped") - case Exception() as exception: - print(f"timer2: exception={exception}") - case None: - # All good, no exception, we can use `selected.value` safely - print( - f"timer2: now={datetime.datetime.now()} " - f"drift={selected.value}" - ) - case _ as unhanded: - assert_never(unhanded) - else: - # This is not necessary, as select() will check for exhaustiveness, but - # it is good practice to have it in case you forgot to handle a new - # receiver added to `select()` at a later point in time. - assert False + async for selected in selector(timer1, timer2): + if selected_from(selected, timer1): + # Beware: `selected.value` might raise an exception, you can always + # check for exceptions with `selected.exception` first or use + # a try-except block. You can also quickly check if the receiver was + # stopped and let any other unexpected exceptions bubble up. + if selected.was_stopped(): + print("timer1 was stopped") + continue + print(f"timer1: now={datetime.datetime.now()} drift={selected.value}") + timer2.stop() + elif selected_from(selected, timer2): + # Explicitly handling of exceptions + match selected.exception: + case ReceiverStoppedError(): + print("timer2 was stopped") + case Exception() as exception: + print(f"timer2: exception={exception}") + case None: + # All good, no exception, we can use `selected.value` safely + print( + f"timer2: now={datetime.datetime.now()} " + f"drift={selected.value}" + ) + case _ as unhanded: + assert_never(unhanded) + else: + # This is not necessary, as select() will check for exhaustiveness, but + # it is good practice to have it in case you forgot to handle a new + # receiver added to `select()` at a later point in time. + assert False ``` ## New Features -* A new `Selector` class was added, please look at the *Upgrading* section for details. +* A new `select()` function was added, please look at the *Upgrading* section for details. * A new `Event` utility receiver was added. @@ -80,14 +79,13 @@ The minimum Python supported version was bumped to 3.11 and the `Select` class r asyncio.ensure_future(exit_after_10_seconds()) - async with Selector(exit_event, other_receiver) as selector: - async for selected in selector: - if selected_from(selected, exit_event): - break - if selected_from(selected, other_receiver): - print(selected.value) - else: - assert False, "Unknow receiver selected" + async for selected in selector(exit_event, other_receiver): + if selected_from(selected, exit_event): + break + if selected_from(selected, other_receiver): + print(selected.value) + else: + assert False, "Unknow receiver selected" ``` * The `Timer` class now has more descriptive `__str__` and `__repr__` methods.