From c66618c02b569ca8c6dc75b54e24de3df3853ee2 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 20 Dec 2024 12:44:58 +0100 Subject: [PATCH 01/18] Add missing cross-referencing for frequenz-quantities Signed-off-by: Leandro Lucarella --- mkdocs.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/mkdocs.yml b/mkdocs.yml index 5a6343e20..bb59da720 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -119,6 +119,7 @@ plugins: - https://docs.python.org/3/objects.inv - https://frequenz-floss.github.io/frequenz-channels-python/v1.1/objects.inv - https://frequenz-floss.github.io/frequenz-client-microgrid-python/v0.5/objects.inv + - https://frequenz-floss.github.io/frequenz-quantities-python/v1/objects.inv - https://lovasoa.github.io/marshmallow_dataclass/html/objects.inv - https://marshmallow.readthedocs.io/en/stable/objects.inv - https://networkx.org/documentation/stable/objects.inv From 961b4fd2dd9bc931739bb4078fca79811bc707b6 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 22 Nov 2024 13:17:37 +0100 Subject: [PATCH 02/18] Bump the minimum required channels version to v1.4.0 This is to be able to use type guards in with `Receiver.filter` to narrow the received type and the new `WithPrevious` class. Signed-off-by: Leandro Lucarella --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 84f91fb7a..07db7ec80 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -30,7 +30,7 @@ dependencies = [ # changing the version # (plugins.mkdocstrings.handlers.python.import) "frequenz-client-microgrid >= 0.6.0, < 0.7.0", - "frequenz-channels >= 1.2.0, < 2.0.0", + "frequenz-channels >= 1.4.0, < 2.0.0", "frequenz-quantities >= 1.0.0rc3, < 2.0.0", "networkx >= 2.8, < 4", "numpy >= 1.26.4, < 2", From 82b02b9cebb181503da934a4f94c2a9d24b3c2f1 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 15 Nov 2024 13:18:21 +0100 Subject: [PATCH 03/18] Add a `ConfigManager` class This class instantiates and starts the `ConfigManagingActor` and creates the channel needed to communicate with it. It offers a convenience method to get receivers to receive configuration updates. Signed-off-by: Leandro Lucarella --- src/frequenz/sdk/config/__init__.py | 4 +- src/frequenz/sdk/config/_manager.py | 118 ++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 src/frequenz/sdk/config/_manager.py diff --git a/src/frequenz/sdk/config/__init__.py b/src/frequenz/sdk/config/__init__.py index 50a16cb79..327777d31 100644 --- a/src/frequenz/sdk/config/__init__.py +++ b/src/frequenz/sdk/config/__init__.py @@ -1,13 +1,15 @@ # License: MIT # Copyright © 2024 Frequenz Energy-as-a-Service GmbH -"""Read and update config variables.""" +"""Configuration management.""" from ._logging_actor import LoggerConfig, LoggingConfig, LoggingConfigUpdatingActor +from ._manager import ConfigManager from ._managing_actor import ConfigManagingActor from ._util import load_config __all__ = [ + "ConfigManager", "ConfigManagingActor", "LoggerConfig", "LoggingConfig", diff --git a/src/frequenz/sdk/config/_manager.py b/src/frequenz/sdk/config/_manager.py new file mode 100644 index 000000000..34b407bff --- /dev/null +++ b/src/frequenz/sdk/config/_manager.py @@ -0,0 +1,118 @@ +# License: MIT +# Copyright © 2024 Frequenz Energy-as-a-Service GmbH + +"""Management of configuration.""" + +import pathlib +from collections.abc import Mapping, Sequence +from datetime import timedelta +from typing import Any, Final + +from frequenz.channels import Broadcast, Receiver +from typing_extensions import override + +from ..actor._background_service import BackgroundService +from ._managing_actor import ConfigManagingActor + + +class ConfigManager(BackgroundService): + """A manager for configuration files. + + This class reads configuration files and sends the configuration to the receivers, + providing optional configuration key filtering and schema validation. + """ + + def __init__( # pylint: disable=too-many-arguments + self, + config_paths: Sequence[pathlib.Path], + /, + *, + force_polling: bool = True, + name: str | None = None, + polling_interval: timedelta = timedelta(seconds=1), + ) -> None: + """Initialize this config manager. + + Args: + config_paths: The paths to the TOML files with the configuration. Order + matters, as the configuration will be read and updated in the order + of the paths, so the last path will override the configuration set by + the previous paths. Dict keys will be merged recursively, but other + objects (like lists) will be replaced by the value in the last path. + force_polling: Whether to force file polling to check for changes. + name: A name to use when creating actors. If `None`, `str(id(self))` will + be used. This is used mostly for debugging purposes. + polling_interval: The interval to poll for changes. Only relevant if + polling is enabled. + """ + super().__init__(name=name) + + self.config_channel: Final[Broadcast[Mapping[str, Any]]] = Broadcast( + name=f"{self}_config", resend_latest=True + ) + """The broadcast channel for the configuration.""" + + self.actor: Final[ConfigManagingActor] = ConfigManagingActor( + config_paths, + self.config_channel.new_sender(), + name=self.name, + force_polling=force_polling, + polling_interval=polling_interval, + ) + """The actor that manages the configuration.""" + + @override + def start(self) -> None: + """Start this config manager.""" + self.actor.start() + + @property + @override + def is_running(self) -> bool: + """Whether this config manager is running.""" + return self.actor.is_running + + @override + def cancel(self, msg: str | None = None) -> None: + """Cancel all running tasks and actors spawned by this config manager. + + Args: + msg: The message to be passed to the tasks being cancelled. + """ + self.actor.cancel(msg) + + # We need the noqa because the `BaseExceptionGroup` is raised indirectly. + @override + async def wait(self) -> None: # noqa: DOC502 + """Wait this config manager to finish. + + Wait until all tasks and actors are finished. + + Raises: + BaseExceptionGroup: If any of the tasks spawned by this service raised an + exception (`CancelError` is not considered an error and not returned in + the exception group). + """ + await self.actor + + @override + def __repr__(self) -> str: + """Return a string representation of this config manager.""" + return f"config_channel={self.config_channel!r}, " f"actor={self.actor!r}>" + + async def new_receiver(self) -> Receiver[Mapping[str, Any] | None]: + """Create a new receiver for the configuration. + + Note: + If there is a burst of configuration updates, the receiver will only + receive the last configuration, older configurations will be ignored. + + Example: + ```python + # TODO: Add Example + ``` + + Returns: + The receiver for the configuration. + """ + return self.config_channel.new_receiver(name=str(self), limit=1) From 723f654cd197e4a4667baf4a008ff9d4487ba521 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 22 Nov 2024 13:07:34 +0100 Subject: [PATCH 04/18] Add option to only send changed configurations When this option is enabled, configurations will be sent to the receiver only if they are different from the previously received configuration. Signed-off-by: Leandro Lucarella --- src/frequenz/sdk/config/_manager.py | 50 ++++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 5 deletions(-) diff --git a/src/frequenz/sdk/config/_manager.py b/src/frequenz/sdk/config/_manager.py index 34b407bff..7a89a58a4 100644 --- a/src/frequenz/sdk/config/_manager.py +++ b/src/frequenz/sdk/config/_manager.py @@ -3,17 +3,21 @@ """Management of configuration.""" +import logging import pathlib from collections.abc import Mapping, Sequence from datetime import timedelta from typing import Any, Final from frequenz.channels import Broadcast, Receiver +from frequenz.channels.experimental import WithPrevious from typing_extensions import override from ..actor._background_service import BackgroundService from ._managing_actor import ConfigManagingActor +_logger = logging.getLogger(__name__) + class ConfigManager(BackgroundService): """A manager for configuration files. @@ -100,19 +104,55 @@ def __repr__(self) -> str: """Return a string representation of this config manager.""" return f"config_channel={self.config_channel!r}, " f"actor={self.actor!r}>" - async def new_receiver(self) -> Receiver[Mapping[str, Any] | None]: + def new_receiver( + self, + *, + skip_unchanged: bool = True, + ) -> Receiver[Mapping[str, Any] | None]: """Create a new receiver for the configuration. - Note: - If there is a burst of configuration updates, the receiver will only - receive the last configuration, older configurations will be ignored. + This method has a lot of features and functionalities to make it easier to + receive configurations, but it also imposes some restrictions on how the + configurations are received. If you need more control over the configuration + receiver, you can create a receiver directly using + [`config_channel.new_receiver()`][frequenz.sdk.config.ConfigManager.config_channel]. + + ### Skipping superfluous updates + + If there is a burst of configuration updates, the receiver will only receive the + last configuration, older configurations will be ignored. + + If `skip_unchanged` is set to `True`, then a configuration that didn't change + compared to the last one received will be ignored and not sent to the receiver. + The comparison is done using the *raw* `dict` to determine if the configuration + has changed. Example: ```python # TODO: Add Example ``` + Args: + skip_unchanged: Whether to skip sending the configuration if it hasn't + changed compared to the last one received. + Returns: The receiver for the configuration. """ - return self.config_channel.new_receiver(name=str(self), limit=1) + receiver = self.config_channel.new_receiver(name=str(self), limit=1) + + if skip_unchanged: + receiver = receiver.filter(WithPrevious(not_equal_with_logging)) + + return receiver + + +def not_equal_with_logging( + old_config: Mapping[str, Any], new_config: Mapping[str, Any] +) -> bool: + """Return whether the two mappings are not equal, logging if they are the same.""" + if old_config == new_config: + _logger.info("Configuration has not changed, skipping update") + _logger.debug("Old configuration being kept: %r", old_config) + return False + return True From f2047077df310c743f5dfa437269330c94356d5e Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 22 Nov 2024 13:21:12 +0100 Subject: [PATCH 05/18] Subscribe to specific configuration keys This forces a better configuration structure, by always having separate sections for separate entities, and avoids spurious updates and noise in the received configuration for other parties. Signed-off-by: Leandro Lucarella --- src/frequenz/sdk/config/__init__.py | 3 +- src/frequenz/sdk/config/_manager.py | 95 ++++++++++++++++++++++++++--- 2 files changed, 88 insertions(+), 10 deletions(-) diff --git a/src/frequenz/sdk/config/__init__.py b/src/frequenz/sdk/config/__init__.py index 327777d31..a889911c3 100644 --- a/src/frequenz/sdk/config/__init__.py +++ b/src/frequenz/sdk/config/__init__.py @@ -4,13 +4,14 @@ """Configuration management.""" from ._logging_actor import LoggerConfig, LoggingConfig, LoggingConfigUpdatingActor -from ._manager import ConfigManager +from ._manager import ConfigManager, InvalidValueForKeyError from ._managing_actor import ConfigManagingActor from ._util import load_config __all__ = [ "ConfigManager", "ConfigManagingActor", + "InvalidValueForKeyError", "LoggerConfig", "LoggingConfig", "LoggingConfigUpdatingActor", diff --git a/src/frequenz/sdk/config/_manager.py b/src/frequenz/sdk/config/_manager.py index 7a89a58a4..dc227accc 100644 --- a/src/frequenz/sdk/config/_manager.py +++ b/src/frequenz/sdk/config/_manager.py @@ -19,6 +19,26 @@ _logger = logging.getLogger(__name__) +class InvalidValueForKeyError(ValueError): + """An error indicating that the value under the specified key is invalid.""" + + def __init__(self, msg: str, *, key: str, value: Any) -> None: + """Initialize this error. + + Args: + msg: The error message. + key: The key that has an invalid value. + value: The actual value that was found that is not a mapping. + """ + super().__init__(msg) + + self.key: Final[Sequence[str]] = key + """The key that has an invalid value.""" + + self.value: Final[Any] = value + """The actual value that was found that is not a mapping.""" + + class ConfigManager(BackgroundService): """A manager for configuration files. @@ -106,10 +126,12 @@ def __repr__(self) -> str: def new_receiver( self, + key: str, + /, *, skip_unchanged: bool = True, - ) -> Receiver[Mapping[str, Any] | None]: - """Create a new receiver for the configuration. + ) -> Receiver[Mapping[str, Any] | InvalidValueForKeyError | None]: + """Create a new receiver for receiving the configuration for a particular key. This method has a lot of features and functionalities to make it easier to receive configurations, but it also imposes some restrictions on how the @@ -117,6 +139,15 @@ def new_receiver( receiver, you can create a receiver directly using [`config_channel.new_receiver()`][frequenz.sdk.config.ConfigManager.config_channel]. + ### Filtering + + Only the configuration under the `key` will be received by the receiver. If the + `key` is not found in the configuration, the receiver will receive `None`. + + The value under `key` must be another mapping, otherwise an error + will be logged and a [`frequenz.sdk.config.InvalidValueForKeyError`][] instance + will be sent to the receiver. + ### Skipping superfluous updates If there is a burst of configuration updates, the receiver will only receive the @@ -133,26 +164,72 @@ def new_receiver( ``` Args: + key: The configuration key to be read by the receiver. skip_unchanged: Whether to skip sending the configuration if it hasn't changed compared to the last one received. Returns: The receiver for the configuration. """ - receiver = self.config_channel.new_receiver(name=str(self), limit=1) + receiver = self.config_channel.new_receiver(name=f"{self}:{key}", limit=1) + + def _get_key_or_error( + config: Mapping[str, Any] + ) -> Mapping[str, Any] | InvalidValueForKeyError | None: + try: + return _get_key(config, key) + except InvalidValueForKeyError as error: + return error + + key_receiver = receiver.map(_get_key_or_error) if skip_unchanged: - receiver = receiver.filter(WithPrevious(not_equal_with_logging)) + return key_receiver.filter(WithPrevious(_not_equal_with_logging)) - return receiver + return key_receiver -def not_equal_with_logging( - old_config: Mapping[str, Any], new_config: Mapping[str, Any] +def _not_equal_with_logging( + old_value: Mapping[str, Any] | InvalidValueForKeyError | None, + new_value: Mapping[str, Any] | InvalidValueForKeyError | None, ) -> bool: """Return whether the two mappings are not equal, logging if they are the same.""" - if old_config == new_config: + if old_value == new_value: _logger.info("Configuration has not changed, skipping update") - _logger.debug("Old configuration being kept: %r", old_config) return False + + if isinstance(new_value, InvalidValueForKeyError) and not isinstance( + old_value, InvalidValueForKeyError + ): + _logger.error( + "Configuration for key %r has an invalid value: %r", + new_value.key, + new_value.value, + ) return True + + +def _get_key(config: Mapping[str, Any], key: str) -> Mapping[str, Any] | None: + """Get the value from the configuration under the specified key. + + Args: + config: The configuration to get the value from. + key: The key to get the value for. + + Returns: + The value under the key, or `None` if the key is not found. + + Raises: + InvalidValueForKeyError: If the value under the key is not a mapping. + """ + match config.get(key): + case None: + return None + case Mapping() as value: + return value + case invalid_value: + raise InvalidValueForKeyError( + f"Value for key {key!r} is not a mapping: {invalid_value!r}", + key=key, + value=invalid_value, + ) From efcdc845fb4407f04c77e44426bd0036e2b3dee1 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 25 Nov 2024 09:47:36 +0100 Subject: [PATCH 06/18] Add support to filter a sub-key Configuration can be nested, and actors could have sub-actors that need their own configuration, so we need to support filtering the configuration by a sequence of keys. For example, if the configuration is `{"key": {"subkey": "value"}}`, and the key is `["key", "subkey"]`, then the receiver will get only `"value"`. Signed-off-by: Leandro Lucarella --- src/frequenz/sdk/config/_manager.py | 80 ++++++++++++++++++++++------- 1 file changed, 62 insertions(+), 18 deletions(-) diff --git a/src/frequenz/sdk/config/_manager.py b/src/frequenz/sdk/config/_manager.py index dc227accc..2f57823a3 100644 --- a/src/frequenz/sdk/config/_manager.py +++ b/src/frequenz/sdk/config/_manager.py @@ -22,7 +22,7 @@ class InvalidValueForKeyError(ValueError): """An error indicating that the value under the specified key is invalid.""" - def __init__(self, msg: str, *, key: str, value: Any) -> None: + def __init__(self, msg: str, *, key: Sequence[str], value: Any) -> None: """Initialize this error. Args: @@ -126,7 +126,12 @@ def __repr__(self) -> str: def new_receiver( self, - key: str, + # This is tricky, because a str is also a Sequence[str], if we would use only + # Sequence[str], then a regular string would also be accepted and taken as + # a sequence, like "key" -> ["k", "e", "y"]. We should never remove the str from + # the allowed types without changing Sequence[str] to something more specific, + # like list[str] or tuple[str] (but both have their own problems). + key: str | Sequence[str], /, *, skip_unchanged: bool = True, @@ -144,6 +149,10 @@ def new_receiver( Only the configuration under the `key` will be received by the receiver. If the `key` is not found in the configuration, the receiver will receive `None`. + If the key is a sequence of strings, it will be treated as a nested key and the + receiver will receive the configuration under the nested key. For example + `["key", "subkey"]` will get only `config["key"]["subkey"]`. + The value under `key` must be another mapping, otherwise an error will be logged and a [`frequenz.sdk.config.InvalidValueForKeyError`][] instance will be sent to the receiver. @@ -164,14 +173,16 @@ def new_receiver( ``` Args: - key: The configuration key to be read by the receiver. + key: The configuration key to be read by the receiver. If a sequence of + strings is used, it is used as a sub-key. skip_unchanged: Whether to skip sending the configuration if it hasn't changed compared to the last one received. Returns: The receiver for the configuration. """ - receiver = self.config_channel.new_receiver(name=f"{self}:{key}", limit=1) + recv_name = key if isinstance(key, str) else ":".join(key) + receiver = self.config_channel.new_receiver(name=recv_name, limit=1) def _get_key_or_error( config: Mapping[str, Any] @@ -184,32 +195,54 @@ def _get_key_or_error( key_receiver = receiver.map(_get_key_or_error) if skip_unchanged: - return key_receiver.filter(WithPrevious(_not_equal_with_logging)) + # For some reason the type argument for WithPrevious is not inferred + # correctly, so we need to specify it explicitly. + return key_receiver.filter( + WithPrevious[Mapping[str, Any] | InvalidValueForKeyError | None]( + lambda old, new: _not_equal_with_logging( + key=key, old_value=old, new_value=new + ) + ) + ) return key_receiver def _not_equal_with_logging( + *, + key: str | Sequence[str], old_value: Mapping[str, Any] | InvalidValueForKeyError | None, new_value: Mapping[str, Any] | InvalidValueForKeyError | None, ) -> bool: """Return whether the two mappings are not equal, logging if they are the same.""" if old_value == new_value: - _logger.info("Configuration has not changed, skipping update") + _logger.info("Configuration has not changed for key %r, skipping update.", key) return False if isinstance(new_value, InvalidValueForKeyError) and not isinstance( old_value, InvalidValueForKeyError ): + subkey_str = "" + if key != new_value.key: + subkey_str = f"When looking for sub-key {key!r}: " _logger.error( - "Configuration for key %r has an invalid value: %r", + "%sConfiguration for key %r has an invalid value: %r", + subkey_str, new_value.key, new_value.value, ) return True -def _get_key(config: Mapping[str, Any], key: str) -> Mapping[str, Any] | None: +def _get_key( + config: Mapping[str, Any], + # This is tricky, because a str is also a Sequence[str], if we would use only + # Sequence[str], then a regular string would also be accepted and taken as + # a sequence, like "key" -> ["k", "e", "y"]. We should never remove the str from + # the allowed types without changing Sequence[str] to something more specific, + # like list[str] or tuple[str]. + key: str | Sequence[str], +) -> Mapping[str, Any] | None: """Get the value from the configuration under the specified key. Args: @@ -222,14 +255,25 @@ def _get_key(config: Mapping[str, Any], key: str) -> Mapping[str, Any] | None: Raises: InvalidValueForKeyError: If the value under the key is not a mapping. """ - match config.get(key): - case None: + # We first normalize to a Sequence[str] to make it easier to work with. + if isinstance(key, str): + key = (key,) + value = config + current_path = [] + for subkey in key: + current_path.append(subkey) + if value is None: return None - case Mapping() as value: - return value - case invalid_value: - raise InvalidValueForKeyError( - f"Value for key {key!r} is not a mapping: {invalid_value!r}", - key=key, - value=invalid_value, - ) + match value.get(subkey): + case None: + return None + case Mapping() as new_value: + value = new_value + case invalid_value: + raise InvalidValueForKeyError( + f"Value for key {current_path!r} is not a mapping: {invalid_value!r}", + key=current_path, + value=invalid_value, + ) + value = new_value + return value From 00214e405ecc4ed9f8a2a399e642fbc19a2da761 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 13 Dec 2024 13:28:23 +0100 Subject: [PATCH 07/18] Validate and load configurations using a config dataclass This commit adds support for validating configurations with a schema. This is useful to ensure the configuration is correct and to receive it as an instance of a dataclass. The `new_receiver` method now requires a `config_class` argument that is used to validate the configuration. If the configuration doesn't pass the validation, it will be ignored and an error will be logged. The schema is expected to be a dataclass, which is used to create a marshmallow schema to validate the configuration. To customize the schema, you can use `marshmallow_dataclass` to specify extra metadata. Validation errors are logged and the `ValidationError` instance is sent to the receiver. Signed-off-by: Leandro Lucarella --- src/frequenz/sdk/config/_manager.py | 116 +++++++++++++++++++++++----- 1 file changed, 97 insertions(+), 19 deletions(-) diff --git a/src/frequenz/sdk/config/_manager.py b/src/frequenz/sdk/config/_manager.py index 2f57823a3..ad5b3a7e7 100644 --- a/src/frequenz/sdk/config/_manager.py +++ b/src/frequenz/sdk/config/_manager.py @@ -11,10 +11,12 @@ from frequenz.channels import Broadcast, Receiver from frequenz.channels.experimental import WithPrevious +from marshmallow import Schema, ValidationError from typing_extensions import override from ..actor._background_service import BackgroundService from ._managing_actor import ConfigManagingActor +from ._util import DataclassT, load_config _logger = logging.getLogger(__name__) @@ -124,7 +126,7 @@ def __repr__(self) -> str: """Return a string representation of this config manager.""" return f"config_channel={self.config_channel!r}, " f"actor={self.actor!r}>" - def new_receiver( + def new_receiver( # pylint: disable=too-many-arguments self, # This is tricky, because a str is also a Sequence[str], if we would use only # Sequence[str], then a regular string would also be accepted and taken as @@ -132,10 +134,13 @@ def new_receiver( # the allowed types without changing Sequence[str] to something more specific, # like list[str] or tuple[str] (but both have their own problems). key: str | Sequence[str], + config_class: type[DataclassT], /, *, skip_unchanged: bool = True, - ) -> Receiver[Mapping[str, Any] | InvalidValueForKeyError | None]: + base_schema: type[Schema] | None = None, + marshmallow_load_kwargs: dict[str, Any] | None = None, + ) -> Receiver[DataclassT | Exception | None]: """Create a new receiver for receiving the configuration for a particular key. This method has a lot of features and functionalities to make it easier to @@ -157,6 +162,21 @@ def new_receiver( will be logged and a [`frequenz.sdk.config.InvalidValueForKeyError`][] instance will be sent to the receiver. + ### Schema validation + + The raw configuration received as a `Mapping` will be validated and loaded to + as a `config_class`. The `config_class` class is expected to be + a [`dataclasses.dataclass`][], which is used to create + a [`marshmallow.Schema`][] via the [`marshmallow_dataclass.class_schema`][] + function. + + This means you can customize the schema derived from the configuration + dataclass using [`marshmallow_dataclass`][] to specify extra validation and + options via field metadata. + + Additional arguments can be passed to [`marshmallow.Schema.load`][] using + the `marshmallow_load_kwargs` keyword arguments. + ### Skipping superfluous updates If there is a burst of configuration updates, the receiver will only receive the @@ -167,6 +187,18 @@ def new_receiver( The comparison is done using the *raw* `dict` to determine if the configuration has changed. + ### Error handling + + The value under `key` must be another mapping, otherwise an error + will be logged and a [`frequenz.sdk.config.InvalidValueForKeyError`][] instance + will be sent to the receiver. + + Configurations that don't pass the validation will be logged as an error and + the [`ValidationError`][marshmallow.ValidationError] sent to the receiver. + + Any other unexpected error raised during the configuration loading will be + logged as an error and the error instance sent to the receiver. + Example: ```python # TODO: Add Example @@ -175,44 +207,49 @@ def new_receiver( Args: key: The configuration key to be read by the receiver. If a sequence of strings is used, it is used as a sub-key. + config_class: The class object to use to instantiate a configuration. The + configuration will be validated against this type too using + [`marshmallow_dataclass`][]. skip_unchanged: Whether to skip sending the configuration if it hasn't changed compared to the last one received. + base_schema: An optional class to be used as a base schema for the + configuration class. This allow using custom fields for example. Will be + passed to [`marshmallow_dataclass.class_schema`][]. + marshmallow_load_kwargs: Additional arguments to be passed to + [`marshmallow.Schema.load`][]. Returns: The receiver for the configuration. """ - recv_name = key if isinstance(key, str) else ":".join(key) - receiver = self.config_channel.new_receiver(name=recv_name, limit=1) - - def _get_key_or_error( - config: Mapping[str, Any] - ) -> Mapping[str, Any] | InvalidValueForKeyError | None: - try: - return _get_key(config, key) - except InvalidValueForKeyError as error: - return error - - key_receiver = receiver.map(_get_key_or_error) + receiver = self.config_channel.new_receiver(name=f"{self}:{key}", limit=1).map( + lambda config: _load_config_with_logging_and_errors( + config, + config_class, + key=key, + base_schema=base_schema, + marshmallow_load_kwargs=marshmallow_load_kwargs, + ) + ) if skip_unchanged: # For some reason the type argument for WithPrevious is not inferred # correctly, so we need to specify it explicitly. - return key_receiver.filter( - WithPrevious[Mapping[str, Any] | InvalidValueForKeyError | None]( + return receiver.filter( + WithPrevious[DataclassT | Exception | None]( lambda old, new: _not_equal_with_logging( key=key, old_value=old, new_value=new ) ) ) - return key_receiver + return receiver def _not_equal_with_logging( *, key: str | Sequence[str], - old_value: Mapping[str, Any] | InvalidValueForKeyError | None, - new_value: Mapping[str, Any] | InvalidValueForKeyError | None, + old_value: DataclassT | Exception | None, + new_value: DataclassT | Exception | None, ) -> bool: """Return whether the two mappings are not equal, logging if they are the same.""" if old_value == new_value: @@ -234,6 +271,47 @@ def _not_equal_with_logging( return True +def _load_config_with_logging_and_errors( + config: Mapping[str, Any], + config_class: type[DataclassT], + *, + key: str | Sequence[str], + base_schema: type[Schema] | None = None, + marshmallow_load_kwargs: dict[str, Any] | None = None, +) -> DataclassT | Exception | None: + """Load the configuration for the specified key, logging errors and returning them.""" + try: + sub_config = _get_key(config, key) + if sub_config is None: + _logger.debug("Configuration key %r not found, sending None", key) + return None + + loaded_config = load_config( + config_class, + sub_config, + base_schema=base_schema, + marshmallow_load_kwargs=marshmallow_load_kwargs, + ) + _logger.debug("Received new configuration: %s", loaded_config) + return loaded_config + except InvalidValueForKeyError as error: + if len(key) > 1 and key != error.key: + _logger.error("Error when looking for sub-key %r: %s", key, error) + else: + _logger.error(str(error)) + return error + except ValidationError as error: + _logger.error("The configuration for key %r is invalid: %s", key, error) + return error + except Exception as error: # pylint: disable=broad-except + _logger.exception( + "An unexpected error occurred while loading the configuration for key %r: %s", + key, + error, + ) + return error + + def _get_key( config: Mapping[str, Any], # This is tricky, because a str is also a Sequence[str], if we would use only From 7db4f74498d02dc8cf747dee665bff2a01dfafd5 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Tue, 10 Dec 2024 14:55:52 +0100 Subject: [PATCH 08/18] Improve handling of unknown fields in the configuration If `unkown` is not specified in the `marshmallow_load_kwargs`, it will default to `marshmallow.EXCLUDE` instead of `marshmallow.RAISE`, as this is what most users will need when loading to a dataclass. But when `marshmallow.EXCLUDE` is used, a warning will be logged if there are fields in the configuration that are being excluded, so the user can still be aware of it and catch typos in the configuration file. Signed-off-by: Leandro Lucarella --- src/frequenz/sdk/config/_manager.py | 71 ++++++++++++++++++++++++++++- 1 file changed, 69 insertions(+), 2 deletions(-) diff --git a/src/frequenz/sdk/config/_manager.py b/src/frequenz/sdk/config/_manager.py index ad5b3a7e7..65ddf684f 100644 --- a/src/frequenz/sdk/config/_manager.py +++ b/src/frequenz/sdk/config/_manager.py @@ -9,6 +9,7 @@ from datetime import timedelta from typing import Any, Final +import marshmallow from frequenz.channels import Broadcast, Receiver from frequenz.channels.experimental import WithPrevious from marshmallow import Schema, ValidationError @@ -177,6 +178,14 @@ def new_receiver( # pylint: disable=too-many-arguments Additional arguments can be passed to [`marshmallow.Schema.load`][] using the `marshmallow_load_kwargs` keyword arguments. + If unspecified, the `marshmallow_load_kwargs` will have the `unknown` key set to + [`marshmallow.EXCLUDE`][] (instead of the normal [`marshmallow.RAISE`][] + default). + + But when [`marshmallow.EXCLUDE`][] is used, a warning will still be logged if + there are extra fields in the configuration that are excluded. This is useful, + for example, to catch typos in the configuration file. + ### Skipping superfluous updates If there is a burst of configuration updates, the receiver will only receive the @@ -220,7 +229,22 @@ def new_receiver( # pylint: disable=too-many-arguments Returns: The receiver for the configuration. + + Raises: + ValueError: If the `unknown` option in `marshmallow_load_kwargs` is set to + [`marshmallow.INCLUDE`][]. """ + marshmallow_load_kwargs = ( + {} if marshmallow_load_kwargs is None else marshmallow_load_kwargs.copy() + ) + + if "unknown" not in marshmallow_load_kwargs: + marshmallow_load_kwargs["unknown"] = marshmallow.EXCLUDE + elif marshmallow_load_kwargs["unknown"] == marshmallow.INCLUDE: + raise ValueError( + "The 'unknown' option can't be 'INCLUDE' when loading to a dataclass" + ) + receiver = self.config_channel.new_receiver(name=f"{self}:{key}", limit=1).map( lambda config: _load_config_with_logging_and_errors( config, @@ -286,9 +310,10 @@ def _load_config_with_logging_and_errors( _logger.debug("Configuration key %r not found, sending None", key) return None - loaded_config = load_config( - config_class, + loaded_config = _load_config( sub_config, + config_class, + key=key, base_schema=base_schema, marshmallow_load_kwargs=marshmallow_load_kwargs, ) @@ -355,3 +380,45 @@ def _get_key( ) value = new_value return value + + +def _load_config( + config: Mapping[str, Any], + config_class: type[DataclassT], + *, + key: str | Sequence[str], + base_schema: type[Schema] | None = None, + marshmallow_load_kwargs: dict[str, Any] | None = None, +) -> DataclassT | InvalidValueForKeyError | ValidationError | None: + """Try to load a configuration and log any validation errors.""" + loaded_config = load_config( + config_class, + config, + base_schema=base_schema, + marshmallow_load_kwargs=marshmallow_load_kwargs, + ) + + marshmallow_load_kwargs = ( + {} if marshmallow_load_kwargs is None else marshmallow_load_kwargs.copy() + ) + + unknown = marshmallow_load_kwargs.get("unknown") + if unknown == marshmallow.EXCLUDE: + # When excluding unknown fields we still want to notify the user, as + # this could mean there is a typo in the configuration and some value is + # not being loaded as desired. + marshmallow_load_kwargs["unknown"] = marshmallow.RAISE + try: + load_config( + config_class, + config, + base_schema=base_schema, + marshmallow_load_kwargs=marshmallow_load_kwargs, + ) + except ValidationError as err: + _logger.warning( + "The configuration for key %r has extra fields that will be ignored: %s", + key, + err, + ) + return loaded_config From 2d0888dbc49163f11cc92709084d927ba84ae807 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 6 Dec 2024 15:12:19 +0100 Subject: [PATCH 09/18] Add a base config schema that provides quantities support This schema is provided to use as a default, and might be extended in the future to support more commonly used fields that are not provided by marshmallow by default. To use the quantity schema we need to bump the `frequenz-quantities` dependency and add the optional `marshmallow` dependency. Signed-off-by: Leandro Lucarella --- pyproject.toml | 2 +- src/frequenz/sdk/config/__init__.py | 2 ++ src/frequenz/sdk/config/_base_schema.py | 10 ++++++++++ src/frequenz/sdk/config/_manager.py | 5 +++-- 4 files changed, 16 insertions(+), 3 deletions(-) create mode 100644 src/frequenz/sdk/config/_base_schema.py diff --git a/pyproject.toml b/pyproject.toml index 07db7ec80..10f132936 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,7 +31,7 @@ dependencies = [ # (plugins.mkdocstrings.handlers.python.import) "frequenz-client-microgrid >= 0.6.0, < 0.7.0", "frequenz-channels >= 1.4.0, < 2.0.0", - "frequenz-quantities >= 1.0.0rc3, < 2.0.0", + "frequenz-quantities[marshmallow] >= 1.0.0, < 2.0.0", "networkx >= 2.8, < 4", "numpy >= 1.26.4, < 2", "typing_extensions >= 4.6.1, < 5", diff --git a/src/frequenz/sdk/config/__init__.py b/src/frequenz/sdk/config/__init__.py index a889911c3..32bca913f 100644 --- a/src/frequenz/sdk/config/__init__.py +++ b/src/frequenz/sdk/config/__init__.py @@ -3,12 +3,14 @@ """Configuration management.""" +from ._base_schema import BaseConfigSchema from ._logging_actor import LoggerConfig, LoggingConfig, LoggingConfigUpdatingActor from ._manager import ConfigManager, InvalidValueForKeyError from ._managing_actor import ConfigManagingActor from ._util import load_config __all__ = [ + "BaseConfigSchema", "ConfigManager", "ConfigManagingActor", "InvalidValueForKeyError", diff --git a/src/frequenz/sdk/config/_base_schema.py b/src/frequenz/sdk/config/_base_schema.py new file mode 100644 index 000000000..d25611958 --- /dev/null +++ b/src/frequenz/sdk/config/_base_schema.py @@ -0,0 +1,10 @@ +# License: MIT +# Copyright © 2024 Frequenz Energy-as-a-Service GmbH + +"""Base schema for configuration classes.""" + +from frequenz.quantities.experimental.marshmallow import QuantitySchema + + +class BaseConfigSchema(QuantitySchema): + """A base schema for configuration classes.""" diff --git a/src/frequenz/sdk/config/_manager.py b/src/frequenz/sdk/config/_manager.py index 65ddf684f..3d701f04e 100644 --- a/src/frequenz/sdk/config/_manager.py +++ b/src/frequenz/sdk/config/_manager.py @@ -16,6 +16,7 @@ from typing_extensions import override from ..actor._background_service import BackgroundService +from ._base_schema import BaseConfigSchema from ._managing_actor import ConfigManagingActor from ._util import DataclassT, load_config @@ -139,7 +140,7 @@ def new_receiver( # pylint: disable=too-many-arguments /, *, skip_unchanged: bool = True, - base_schema: type[Schema] | None = None, + base_schema: type[Schema] | None = BaseConfigSchema, marshmallow_load_kwargs: dict[str, Any] | None = None, ) -> Receiver[DataclassT | Exception | None]: """Create a new receiver for receiving the configuration for a particular key. @@ -387,7 +388,7 @@ def _load_config( config_class: type[DataclassT], *, key: str | Sequence[str], - base_schema: type[Schema] | None = None, + base_schema: type[Schema] | None = BaseConfigSchema, marshmallow_load_kwargs: dict[str, Any] | None = None, ) -> DataclassT | InvalidValueForKeyError | ValidationError | None: """Try to load a configuration and log any validation errors.""" From 86072072bf022335f010dc2e54be74e50afd9caa Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 9 Dec 2024 11:57:15 +0100 Subject: [PATCH 10/18] Make the `LoggingConfigUpdatingActor` use the `ConfigManager` The `LoggingConfigUpdatingActor` can now also receive `None` as configuration (for example if the configuration is removed), in which case it will go back to the default configuration. Signed-off-by: Leandro Lucarella --- RELEASE_NOTES.md | 4 +- src/frequenz/sdk/config/_logging_actor.py | 62 ++++++++-------- tests/config/test_logging_actor.py | 86 ++++++++++------------- 3 files changed, 69 insertions(+), 83 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 057a3ef83..7d2ad13fa 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -11,7 +11,9 @@ * `LoggingConfigUpdater` + Renamed to `LoggingConfigUpdatingActor` to follow the actor naming convention. - + Make all arguments to the constructor keyword-only. + + The actor must now be constructed using a `ConfigManager` instead of a receiver. + + Make all arguments to the constructor keyword-only, except for the `config_manager` argument. + + If the configuration is removed, the actor will now load back the default configuration. * `LoggingConfig` diff --git a/src/frequenz/sdk/config/_logging_actor.py b/src/frequenz/sdk/config/_logging_actor.py index d5d75bf5c..e2011bc08 100644 --- a/src/frequenz/sdk/config/_logging_actor.py +++ b/src/frequenz/sdk/config/_logging_actor.py @@ -4,16 +4,14 @@ """Read and update logging severity from config.""" import logging -from collections.abc import Mapping from dataclasses import dataclass, field -from typing import Annotated, Any +from typing import Annotated, Sequence, assert_never import marshmallow import marshmallow.validate -from frequenz.channels import Receiver from ..actor import Actor -from ._util import load_config +from ._manager import ConfigManager _logger = logging.getLogger(__name__) @@ -84,26 +82,13 @@ class LoggingConfigUpdatingActor(Actor): ```python import asyncio - from collections.abc import Mapping - from typing import Any - from frequenz.channels import Broadcast - from frequenz.sdk.config import LoggingConfigUpdatingActor, ConfigManagingActor + from frequenz.sdk.config import LoggingConfigUpdatingActor from frequenz.sdk.actor import run as run_actors async def run() -> None: - config_channel = Broadcast[Mapping[str, Any]](name="config", resend_latest=True) - actors = [ - ConfigManagingActor( - config_paths=["config.toml"], output=config_channel.new_sender() - ), - LoggingConfigUpdatingActor( - config_recv=config_channel.new_receiver(limit=1)).map( - lambda app_config: app_config.get("logging", {} - ) - ), - ] - await run_actors(*actors) + config_manager: ConfigManager = ... + await run_actors(LoggingConfigUpdatingActor(config_manager)) asyncio.run(run()) ``` @@ -112,10 +97,13 @@ async def run() -> None: will be updated as well. """ + # pylint: disable-next=too-many-arguments def __init__( self, + config_manager: ConfigManager, + /, *, - config_recv: Receiver[Mapping[str, Any]], + config_key: str | Sequence[str] = "logging", log_datefmt: str = "%Y-%m-%dT%H:%M:%S%z", log_format: str = "%(asctime)s %(levelname)-8s %(name)s:%(lineno)s: %(message)s", name: str | None = None, @@ -123,7 +111,9 @@ def __init__( """Initialize this instance. Args: - config_recv: The receiver to listen for configuration changes. + config_manager: The configuration manager to use. + config_key: The key to use to retrieve the configuration from the + configuration manager. If `None`, the whole configuration will be used. log_datefmt: Use the specified date/time format in logs. log_format: Use the specified format string in logs. name: The name of this actor. If `None`, `str(id(self))` will be used. This @@ -135,7 +125,9 @@ def __init__( in the application (through a previous `basicConfig()` call), then the format settings specified here will be ignored. """ - self._config_recv = config_recv + self._config_receiver = config_manager.new_receiver( + config_key, LoggingConfig, base_schema=None + ) # Setup default configuration. # This ensures logging is configured even if actor fails to start or @@ -153,17 +145,19 @@ def __init__( async def _run(self) -> None: """Listen for configuration changes and update logging.""" - async for message in self._config_recv: - try: - new_config = load_config(LoggingConfig, message) - except marshmallow.ValidationError: - _logger.exception( - "Invalid logging configuration received. Skipping config update" - ) - continue - - if new_config != self._current_config: - self._update_logging(new_config) + async for new_config in self._config_receiver: + match new_config: + case None: + # When we receive None, we want to reset the logging configuration + # to the default + self._update_logging(LoggingConfig()) + case LoggingConfig(): + self._update_logging(new_config) + case Exception(): + # We ignore errors and just keep the old configuration + pass + case unexpected: + assert_never(unexpected) def _update_logging(self, config: LoggingConfig) -> None: """Configure the logging level.""" diff --git a/tests/config/test_logging_actor.py b/tests/config/test_logging_actor.py index 75c1e256c..71b020b41 100644 --- a/tests/config/test_logging_actor.py +++ b/tests/config/test_logging_actor.py @@ -5,7 +5,6 @@ import asyncio import logging -from collections.abc import Mapping from typing import Any import pytest @@ -77,78 +76,69 @@ async def test_logging_config_updating_actor( # is not working anyway - python ignores it. mocker.patch("frequenz.sdk.config._logging_actor.logging.basicConfig") - config_channel = Broadcast[Mapping[str, Any]](name="config") - config_sender = config_channel.new_sender() - async with LoggingConfigUpdatingActor( - config_recv=config_channel.new_receiver().map( - lambda app_config: app_config.get("logging", {}) - ) - ) as actor: + # Mock ConfigManager + mock_config_manager = mocker.Mock() + mock_config_manager.config_channel = Broadcast[LoggingConfig | Exception | None]( + name="config" + ) + mock_config_manager.new_receiver = mocker.Mock( + return_value=mock_config_manager.config_channel.new_receiver() + ) + + async with LoggingConfigUpdatingActor(mock_config_manager) as actor: assert logging.getLogger("frequenz.sdk.actor").level == logging.NOTSET assert logging.getLogger("frequenz.sdk.timeseries").level == logging.NOTSET update_logging_spy = mocker.spy(actor, "_update_logging") # Send first config - await config_sender.send( - { - "logging": { - "root_logger": {"level": "ERROR"}, - "loggers": { - "frequenz.sdk.actor": {"level": "DEBUG"}, - "frequenz.sdk.timeseries": {"level": "ERROR"}, - }, - } - } + expected_config = LoggingConfig( + root_logger=LoggerConfig(level="ERROR"), + loggers={ + "frequenz.sdk.actor": LoggerConfig(level="DEBUG"), + "frequenz.sdk.timeseries": LoggerConfig(level="ERROR"), + }, ) + await mock_config_manager.config_channel.new_sender().send(expected_config) await asyncio.sleep(0.01) - update_logging_spy.assert_called_once_with( - LoggingConfig( - root_logger=LoggerConfig(level="ERROR"), - loggers={ - "frequenz.sdk.actor": LoggerConfig(level="DEBUG"), - "frequenz.sdk.timeseries": LoggerConfig(level="ERROR"), - }, - ) - ) + update_logging_spy.assert_called_once_with(expected_config) assert logging.getLogger("frequenz.sdk.actor").level == logging.DEBUG assert logging.getLogger("frequenz.sdk.timeseries").level == logging.ERROR update_logging_spy.reset_mock() - # Update config - await config_sender.send( - { - "logging": { - "root_logger": {"level": "WARNING"}, - "loggers": { - "frequenz.sdk.actor": {"level": "INFO"}, - }, - } - } + # Send an exception and verify the previous config is maintained + await mock_config_manager.config_channel.new_sender().send( + ValueError("Test error") ) await asyncio.sleep(0.01) + update_logging_spy.assert_not_called() # Should not try to update logging + # Previous config should be maintained + assert logging.getLogger("frequenz.sdk.actor").level == logging.DEBUG + assert logging.getLogger("frequenz.sdk.timeseries").level == logging.ERROR + assert ( + actor._current_config == expected_config # pylint: disable=protected-access + ) # pylint: disable=protected-access + update_logging_spy.reset_mock() + + # Update config expected_config = LoggingConfig( root_logger=LoggerConfig(level="WARNING"), loggers={ "frequenz.sdk.actor": LoggerConfig(level="INFO"), }, ) + await mock_config_manager.config_channel.new_sender().send(expected_config) + await asyncio.sleep(0.01) update_logging_spy.assert_called_once_with(expected_config) assert logging.getLogger("frequenz.sdk.actor").level == logging.INFO assert logging.getLogger("frequenz.sdk.timeseries").level == logging.NOTSET update_logging_spy.reset_mock() - # Send invalid config to make sure actor doesn't crash and doesn't setup invalid config. - await config_sender.send({"logging": {"root_logger": {"level": "UNKNOWN"}}}) - await asyncio.sleep(0.01) - update_logging_spy.assert_not_called() - assert actor._current_config == expected_config - update_logging_spy.reset_mock() - - # Send empty config to reset logging to default - await config_sender.send({"other": {"var1": 1}}) + # Send a None config to make sure actor doesn't crash and configures a default logging + await mock_config_manager.config_channel.new_sender().send(None) await asyncio.sleep(0.01) update_logging_spy.assert_called_once_with(LoggingConfig()) - assert logging.getLogger("frequenz.sdk.actor").level == logging.NOTSET - assert logging.getLogger("frequenz.sdk.timeseries").level == logging.NOTSET + assert ( + actor._current_config == LoggingConfig() # pylint: disable=protected-access + ) update_logging_spy.reset_mock() From 8766ba8b4291fb84b8dd795824a6daa1d3d77fe6 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 9 Dec 2024 13:56:48 +0100 Subject: [PATCH 11/18] Allow configuring logging via `ConfigManager` Signed-off-by: Leandro Lucarella --- src/frequenz/sdk/config/_manager.py | 59 +++++++++++++++++++++++++---- 1 file changed, 51 insertions(+), 8 deletions(-) diff --git a/src/frequenz/sdk/config/_manager.py b/src/frequenz/sdk/config/_manager.py index 3d701f04e..ad27d120b 100644 --- a/src/frequenz/sdk/config/_manager.py +++ b/src/frequenz/sdk/config/_manager.py @@ -56,6 +56,7 @@ def __init__( # pylint: disable=too-many-arguments /, *, force_polling: bool = True, + logging_config_key: str | Sequence[str] | None = "logging", name: str | None = None, polling_interval: timedelta = timedelta(seconds=1), ) -> None: @@ -68,6 +69,10 @@ def __init__( # pylint: disable=too-many-arguments the previous paths. Dict keys will be merged recursively, but other objects (like lists) will be replaced by the value in the last path. force_polling: Whether to force file polling to check for changes. + logging_config_key: The key to use for the logging configuration. If `None`, + logging configuration will not be managed. If a key is provided, the + manager update the logging configuration whenever the configuration + changes. name: A name to use when creating actors. If `None`, `str(id(self))` will be used. This is used mostly for debugging purposes. polling_interval: The interval to poll for changes. Only relevant if @@ -80,7 +85,7 @@ def __init__( # pylint: disable=too-many-arguments ) """The broadcast channel for the configuration.""" - self.actor: Final[ConfigManagingActor] = ConfigManagingActor( + self.config_actor: Final[ConfigManagingActor] = ConfigManagingActor( config_paths, self.config_channel.new_sender(), name=self.name, @@ -89,16 +94,31 @@ def __init__( # pylint: disable=too-many-arguments ) """The actor that manages the configuration.""" + # pylint: disable-next=import-outside-toplevel,cyclic-import + from ._logging_actor import LoggingConfigUpdatingActor + + self.logging_actor: Final[LoggingConfigUpdatingActor | None] = ( + None + if logging_config_key is None + else LoggingConfigUpdatingActor( + self, config_key=logging_config_key, name=self.name + ) + ) + @override def start(self) -> None: """Start this config manager.""" - self.actor.start() + self.config_actor.start() + if self.logging_actor: + self.logging_actor.start() @property @override def is_running(self) -> bool: """Whether this config manager is running.""" - return self.actor.is_running + return self.config_actor.is_running or ( + self.logging_actor is not None and self.logging_actor.is_running + ) @override def cancel(self, msg: str | None = None) -> None: @@ -107,11 +127,12 @@ def cancel(self, msg: str | None = None) -> None: Args: msg: The message to be passed to the tasks being cancelled. """ - self.actor.cancel(msg) + if self.logging_actor: + self.logging_actor.cancel(msg) + self.config_actor.cancel(msg) - # We need the noqa because the `BaseExceptionGroup` is raised indirectly. @override - async def wait(self) -> None: # noqa: DOC502 + async def wait(self) -> None: """Wait this config manager to finish. Wait until all tasks and actors are finished. @@ -121,12 +142,34 @@ async def wait(self) -> None: # noqa: DOC502 exception (`CancelError` is not considered an error and not returned in the exception group). """ - await self.actor + exceptions: list[BaseException] = [] + if self.logging_actor: + try: + await self.logging_actor + except BaseExceptionGroup as err: # pylint: disable=try-except-raise + exceptions.append(err) + + try: + await self.config_actor + except BaseExceptionGroup as err: # pylint: disable=try-except-raise + exceptions.append(err) + + if exceptions: + raise BaseExceptionGroup(f"Error while stopping {self!r}", exceptions) @override def __repr__(self) -> str: """Return a string representation of this config manager.""" - return f"config_channel={self.config_channel!r}, " f"actor={self.actor!r}>" + logging_actor = ( + f"logging_actor={self.logging_actor!r}, " if self.logging_actor else "" + ) + return ( + f"<{self.__class__.__name__}: " + f"name={self.name!r}, " + f"config_channel={self.config_channel!r}, " + + logging_actor + + f"config_actor={self.config_actor!r}>" + ) def new_receiver( # pylint: disable=too-many-arguments self, From 414f64d08fa6321befb6a8df6db408ddfe0f5e10 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Tue, 17 Dec 2024 09:43:06 +0100 Subject: [PATCH 12/18] Make `ConfigManagingActor` logging less redundant We were using `str(self)` as prefix, which prints the class name, but the logger name already has enough information to figure out this log comes from the config managing actor, so we now use only the unique ID (name) of the actor in the logs. Signed-off-by: Leandro Lucarella --- src/frequenz/sdk/config/_managing_actor.py | 36 ++++++++++++---------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/src/frequenz/sdk/config/_managing_actor.py b/src/frequenz/sdk/config/_managing_actor.py index 4bb322803..f0b00a71e 100644 --- a/src/frequenz/sdk/config/_managing_actor.py +++ b/src/frequenz/sdk/config/_managing_actor.py @@ -117,24 +117,26 @@ def _read_config(self) -> abc.Mapping[str, Any] | None: config: dict[str, Any] = {} for config_path in self._config_paths: - _logger.info("%s: Reading configuration file %s...", self, config_path) + _logger.info( + "[%s] Reading configuration file %r...", self.name, str(config_path) + ) try: with config_path.open("rb") as toml_file: data = tomllib.load(toml_file) _logger.info( - "%s: Configuration file %r read successfully.", - self, + "[%s] Configuration file %r read successfully.", + self.name, str(config_path), ) config = _recursive_update(config, data) except ValueError as err: - _logger.error("%s: Can't read config file, err: %s", self, err) + _logger.error("[%s] Can't read config file, err: %s", self.name, err) error_count += 1 except OSError as err: # It is ok for config file to don't exist. _logger.error( - "%s: Error reading config file %r (%s). Ignoring it.", - self, + "[%s] Error reading config file %r (%s). Ignoring it.", + self.name, str(config_path), err, ) @@ -142,13 +144,13 @@ def _read_config(self) -> abc.Mapping[str, Any] | None: if error_count == len(self._config_paths): _logger.error( - "%s: Can't read any of the config files, ignoring config update.", self + "[%s] Can't read any of the config files, ignoring config update.", self ) return None _logger.info( - "%s: Read %s/%s configuration files successfully.", - self, + "[%s] Read %s/%s configuration files successfully.", + self.name, len(self._config_paths) - error_count, len(self._config_paths), ) @@ -185,8 +187,8 @@ async def _run(self) -> None: async for event in file_watcher: if not event.path.exists(): _logger.error( - "%s: Received event %s, but the watched path %s doesn't exist.", - self, + "[%s] Received event %s, but the watched path %s doesn't exist.", + self.name, event, event.path, ) @@ -207,23 +209,23 @@ async def _run(self) -> None: match event.type: case EventType.CREATE: _logger.info( - "%s: The configuration file %s was created, sending new config...", - self, + "[%s] The configuration file %s was created, sending new config...", + self.name, event.path, ) await self.send_config() case EventType.MODIFY: _logger.info( - "%s: The configuration file %s was modified, sending update...", - self, + "[%s] The configuration file %s was modified, sending update...", + self.name, event.path, ) await self.send_config() case EventType.DELETE: _logger.error( - "%s: Unexpected DELETE event for path %s. Please report this " + "[%s] Unexpected DELETE event for path %s. Please report this " "issue to Frequenz.", - self, + self.name, event.path, ) case _: From 177af6a44a4eafd588a9e4fd9f30c3214804c429 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Tue, 17 Dec 2024 09:46:36 +0100 Subject: [PATCH 13/18] Add a new utility function to wait for the first config This function has a default timeout, so waiting for the first configuration can be automatically aborted when it takes too long. The method will also reports some progress, logging errors if there were issues receiving or loading the configuration, or if the configuration is missing, setting a logging baseline for all actors. Signed-off-by: Leandro Lucarella --- src/frequenz/sdk/config/__init__.py | 3 +- src/frequenz/sdk/config/_manager.py | 99 ++++++++++++++++++++++++++++- 2 files changed, 99 insertions(+), 3 deletions(-) diff --git a/src/frequenz/sdk/config/__init__.py b/src/frequenz/sdk/config/__init__.py index 32bca913f..8355ac7f7 100644 --- a/src/frequenz/sdk/config/__init__.py +++ b/src/frequenz/sdk/config/__init__.py @@ -5,7 +5,7 @@ from ._base_schema import BaseConfigSchema from ._logging_actor import LoggerConfig, LoggingConfig, LoggingConfigUpdatingActor -from ._manager import ConfigManager, InvalidValueForKeyError +from ._manager import ConfigManager, InvalidValueForKeyError, wait_for_first from ._managing_actor import ConfigManagingActor from ._util import load_config @@ -18,4 +18,5 @@ "LoggingConfig", "LoggingConfigUpdatingActor", "load_config", + "wait_for_first", ] diff --git a/src/frequenz/sdk/config/_manager.py b/src/frequenz/sdk/config/_manager.py index ad27d120b..1670987fb 100644 --- a/src/frequenz/sdk/config/_manager.py +++ b/src/frequenz/sdk/config/_manager.py @@ -3,14 +3,16 @@ """Management of configuration.""" +import asyncio import logging import pathlib from collections.abc import Mapping, Sequence +from dataclasses import is_dataclass from datetime import timedelta -from typing import Any, Final +from typing import Any, Final, Literal, TypeGuard, overload import marshmallow -from frequenz.channels import Broadcast, Receiver +from frequenz.channels import Broadcast, Receiver, ReceiverStoppedError from frequenz.channels.experimental import WithPrevious from marshmallow import Schema, ValidationError from typing_extensions import override @@ -313,6 +315,99 @@ def new_receiver( # pylint: disable=too-many-arguments return receiver +@overload +async def wait_for_first( + receiver: Receiver[DataclassT | Exception | None], + /, + *, + receiver_name: str | None = None, + allow_none: Literal[False] = False, + timeout: timedelta = timedelta(minutes=1), +) -> DataclassT: ... + + +@overload +async def wait_for_first( + receiver: Receiver[DataclassT | Exception | None], + /, + *, + receiver_name: str | None = None, + allow_none: Literal[True] = True, + timeout: timedelta = timedelta(minutes=1), +) -> DataclassT | None: ... + + +async def wait_for_first( + receiver: Receiver[DataclassT | Exception | None], + /, + *, + receiver_name: str | None = None, + allow_none: bool = False, + timeout: timedelta = timedelta(minutes=1), +) -> DataclassT | None: + """Receive the first configuration. + + Args: + receiver: The receiver to receive the first configuration from. + receiver_name: The name of the receiver, used for logging. If `None`, the + string representation of the receiver will be used. + allow_none: Whether consider a `None` value as a valid configuration. + timeout: The timeout in seconds to wait for the first configuration. + + Returns: + The first configuration received. + + Raises: + asyncio.TimeoutError: If the first configuration is not received within the + timeout. + ReceiverStoppedError: If the receiver is stopped before the first configuration + is received. + """ + if receiver_name is None: + receiver_name = str(receiver) + + # We need this type guard because we can't use a TypeVar for isinstance checks or + # match cases. + def is_config_class(value: DataclassT | Exception | None) -> TypeGuard[DataclassT]: + return is_dataclass(value) if value is not None else False + + _logger.info( + "%s: Waiting %s seconds for the first configuration to arrive...", + receiver_name, + timeout.total_seconds(), + ) + try: + async with asyncio.timeout(timeout.total_seconds()): + async for config in receiver: + match config: + case None: + if allow_none: + return None + _logger.error( + "%s: Received empty configuration, waiting again for " + "a first configuration to be set.", + receiver_name, + ) + case Exception() as error: + _logger.error( + "%s: Error while receiving the first configuration, " + "will keep waiting for an update: %s.", + receiver_name, + error, + ) + case config if is_config_class(config): + _logger.info("%s: Received first configuration.", receiver_name) + return config + case unexpected: + assert ( + False + ), f"{receiver_name}: Unexpected value received: {unexpected!r}." + except asyncio.TimeoutError: + _logger.error("%s: No configuration received in time.", receiver_name) + raise + raise ReceiverStoppedError(receiver) + + def _not_equal_with_logging( *, key: str | Sequence[str], From b606d982a6a29ccf1de70e24cf3fefd82c2113ed Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Tue, 17 Dec 2024 09:48:45 +0100 Subject: [PATCH 14/18] Use `wait_for_first` in the `LoggingConfigUpdatingActor` We now wait for the first configuration using the new `wait_for_first()` utility function. We also make the logging more explicity about when the old configuration is being kept and why. Signed-off-by: Leandro Lucarella --- src/frequenz/sdk/config/_logging_actor.py | 52 ++++++++++++++++------- 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/src/frequenz/sdk/config/_logging_actor.py b/src/frequenz/sdk/config/_logging_actor.py index e2011bc08..739e0be07 100644 --- a/src/frequenz/sdk/config/_logging_actor.py +++ b/src/frequenz/sdk/config/_logging_actor.py @@ -11,7 +11,7 @@ import marshmallow.validate from ..actor import Actor -from ._manager import ConfigManager +from ._manager import ConfigManager, wait_for_first _logger = logging.getLogger(__name__) @@ -141,23 +141,45 @@ def __init__( datefmt=log_datefmt, level=logging.INFO, ) - self._update_logging(self._current_config) + _logger.info("Applying initial default logging configuration...") + self._reconfigure(self._current_config) async def _run(self) -> None: """Listen for configuration changes and update logging.""" - async for new_config in self._config_receiver: - match new_config: - case None: - # When we receive None, we want to reset the logging configuration - # to the default - self._update_logging(LoggingConfig()) - case LoggingConfig(): - self._update_logging(new_config) - case Exception(): - # We ignore errors and just keep the old configuration - pass - case unexpected: - assert_never(unexpected) + self._reconfigure( + await wait_for_first( + self._config_receiver, receiver_name=str(self), allow_none=True + ) + ) + async for config_update in self._config_receiver: + self._reconfigure(config_update) + + def _reconfigure(self, config_update: LoggingConfig | Exception | None) -> None: + """Update the logging configuration. + + Args: + config_update: The new configuration, or an exception if there was an error + parsing the configuration, or `None` if the configuration was unset. + """ + match config_update: + case LoggingConfig(): + _logger.info( + "New configuration received, updating logging configuration." + ) + self._update_logging(config_update) + case None: + _logger.info( + "Configuration was unset, resetting to the default " + "logging configuration." + ) + self._update_logging(LoggingConfig()) + case Exception(): + _logger.info( + "New configuration has errors, keeping the old logging " + "configuration." + ) + case unexpected: + assert_never(unexpected) def _update_logging(self, config: LoggingConfig) -> None: """Configure the logging level.""" From e8d1f3a8657255627624567605f1840f0bc66d95 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 20 Dec 2024 12:49:54 +0100 Subject: [PATCH 15/18] Allow passing a single configuration file Both the `ConfigManager` and the `ConfigManagingActor` can now take a single configuration file, both as a `str` or a `Path`. This also fixes a bug where `str` was still accepted, but taken as a `Sequence[str]`. This is a known issue with Python typing: https://github.com/python/mypy/issues/11001 And while we are it, we now also explicitly reject empty sequences, raising a `ValueError` when one is passed. Signed-off-by: Leandro Lucarella --- src/frequenz/sdk/config/_manager.py | 2 +- src/frequenz/sdk/config/_managing_actor.py | 31 +++++++++++++++------- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/frequenz/sdk/config/_manager.py b/src/frequenz/sdk/config/_manager.py index 1670987fb..eb3552984 100644 --- a/src/frequenz/sdk/config/_manager.py +++ b/src/frequenz/sdk/config/_manager.py @@ -54,7 +54,7 @@ class ConfigManager(BackgroundService): def __init__( # pylint: disable=too-many-arguments self, - config_paths: Sequence[pathlib.Path], + config_paths: str | pathlib.Path | Sequence[pathlib.Path | str], /, *, force_polling: bool = True, diff --git a/src/frequenz/sdk/config/_managing_actor.py b/src/frequenz/sdk/config/_managing_actor.py index f0b00a71e..9c91cac21 100644 --- a/src/frequenz/sdk/config/_managing_actor.py +++ b/src/frequenz/sdk/config/_managing_actor.py @@ -72,7 +72,7 @@ class ConfigManagingActor(Actor): # pylint: disable-next=too-many-arguments def __init__( self, - config_paths: abc.Sequence[pathlib.Path | str], + config_paths: str | pathlib.Path | abc.Sequence[pathlib.Path | str], output: Sender[abc.Mapping[str, Any]], *, name: str | None = None, @@ -93,16 +93,29 @@ def __init__( force_polling: Whether to force file polling to check for changes. polling_interval: The interval to poll for changes. Only relevant if polling is enabled. + + Raises: + ValueError: If no configuration path is provided. """ super().__init__(name=name) - self._config_paths: list[pathlib.Path] = [ - ( - config_path - if isinstance(config_path, pathlib.Path) - else pathlib.Path(config_path) - ) - for config_path in config_paths - ] + match config_paths: + case str(): + self._config_paths = [pathlib.Path(config_paths)] + case pathlib.Path(): + self._config_paths = [config_paths] + case abc.Sequence() as seq if len(seq) == 0: + raise ValueError("At least one config path is required.") + case abc.Sequence(): + self._config_paths = [ + ( + config_path + if isinstance(config_path, pathlib.Path) + else pathlib.Path(config_path) + ) + for config_path in config_paths + ] + case unexpected: + assert_never(unexpected) self._output: Sender[abc.Mapping[str, Any]] = output self._force_polling: bool = force_polling self._polling_interval: timedelta = polling_interval From 18442418f50bb916e01f4e254f164baf375b923c Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Thu, 19 Dec 2024 14:55:28 +0100 Subject: [PATCH 16/18] Add tests for the `ConfigManager` Signed-off-by: Leandro Lucarella --- tests/config/test_manager.py | 462 +++++++++++++++++++++++++++++++++++ 1 file changed, 462 insertions(+) create mode 100644 tests/config/test_manager.py diff --git a/tests/config/test_manager.py b/tests/config/test_manager.py new file mode 100644 index 000000000..b36154758 --- /dev/null +++ b/tests/config/test_manager.py @@ -0,0 +1,462 @@ +# License: MIT +# Copyright © 2024 Frequenz Energy-as-a-Service GmbH + +"""Tests for the config manager module.""" + + +import asyncio +import dataclasses +import logging +import pathlib +from collections.abc import Mapping, Sequence +from dataclasses import dataclass +from datetime import timedelta +from typing import Any, assert_never + +import marshmallow +import pytest +import pytest_mock + +from frequenz.sdk.config import ConfigManager, InvalidValueForKeyError, wait_for_first +from frequenz.sdk.config._manager import _get_key + + +@dataclass +class SimpleConfig: + """A simple configuration class for testing.""" + + name: str = dataclasses.field(metadata={"validate": lambda s: s.startswith("test")}) + value: int + + +@dataclass(frozen=True, kw_only=True) +class ReceiverTestCase: + """A test case for testing new_receiver configurations.""" + + title: str + key: str | tuple[str, ...] + config_class: type[SimpleConfig] + input_config: dict[str, Any] + expected_output: Any | None + base_schema: type[marshmallow.Schema] | None = None + marshmallow_load_kwargs: dict[str, Any] | None = None + + +# Test cases for new_receiver +receiver_test_cases = [ + ReceiverTestCase( + title="Basic Config", + key="test", + config_class=SimpleConfig, + input_config={"test": {"name": "test1", "value": 42}}, + expected_output=SimpleConfig(name="test1", value=42), + ), + ReceiverTestCase( + title="Nested Key Config", + key=("nested", "config"), + config_class=SimpleConfig, + input_config={"nested": {"config": {"name": "test2", "value": 43}}}, + expected_output=SimpleConfig(name="test2", value=43), + ), + ReceiverTestCase( + title="Validation Error", + key="test", + config_class=SimpleConfig, + input_config={"test": {"name": "no-test1", "value": 42}}, + expected_output="{'name': ['Invalid value.']}", + ), + ReceiverTestCase( + title="Invalid Value Type", + key="test", + config_class=SimpleConfig, + input_config={"test": "not a mapping"}, + expected_output="Value for key ['test'] is not a mapping: 'not a mapping'", + ), + ReceiverTestCase( + title="Raise on unknown", + key="test", + config_class=SimpleConfig, + marshmallow_load_kwargs={"unknown": marshmallow.RAISE}, + input_config={"test": {"name": "test3", "value": 44, "not_allowed": 42}}, + expected_output="{'not_allowed': ['Unknown field.']}", + ), + ReceiverTestCase( + title="Missing Key", + key="missing", + config_class=SimpleConfig, + input_config={"test": {"name": "test3", "value": 44}}, + expected_output=None, + ), +] + + +@pytest.mark.parametrize("test_case", receiver_test_cases, ids=lambda tc: tc.title) +async def test_new_receiver_configurations( + test_case: ReceiverTestCase, mocker: pytest_mock.MockFixture +) -> None: + """Test different configurations for new_receiver.""" + mocker.patch("frequenz.sdk.config._manager.ConfigManagingActor") + config_manager = ConfigManager([pathlib.Path("dummy.toml")]) + await config_manager.config_channel.new_sender().send(test_case.input_config) + receiver = config_manager.new_receiver( + test_case.key, + test_case.config_class, + base_schema=test_case.base_schema, + marshmallow_load_kwargs=test_case.marshmallow_load_kwargs, + ) + + async with asyncio.timeout(1): + result = await receiver.receive() + match result: + case SimpleConfig() | None: + assert result == test_case.expected_output + case Exception(): + assert str(result) == str(test_case.expected_output) + case unexpected: + assert_never(unexpected) + + +async def test_warn_on_unknown_key( + mocker: pytest_mock.MockerFixture, caplog: pytest.LogCaptureFixture +) -> None: + """Test that a warning is logged when an unknown key is received.""" + mocker.patch("frequenz.sdk.config._manager.ConfigManagingActor") + config_manager = ConfigManager([pathlib.Path("dummy.toml")]) + await config_manager.config_channel.new_sender().send( + {"test": {"name": "test3", "value": 44, "not_allowed": 42}} + ) + receiver = config_manager.new_receiver("test", SimpleConfig) + + async with asyncio.timeout(1): + await receiver.receive() + + expected_log_entry = ( + "frequenz.sdk.config._manager", + logging.WARNING, + "The configuration for key 'test' has extra fields that will be ignored: " + "{'not_allowed': ['Unknown field.']}", + ) + assert expected_log_entry in caplog.record_tuples + + +async def test_skip_config_update_bursts(mocker: pytest_mock.MockerFixture) -> None: + """Test that a burst of updates will only send the last update.""" + mocker.patch("frequenz.sdk.config._manager.ConfigManagingActor") + config_manager = ConfigManager([pathlib.Path("dummy.toml")]) + sender = config_manager.config_channel.new_sender() + receiver = config_manager.new_receiver( + "test", + SimpleConfig, + skip_unchanged=True, + ) + + await sender.send({"test": {"name": "test1", "value": 42}}) + await sender.send({"test": {"name": "test2", "value": 43}}) + await sender.send({"test": {"name": "test3", "value": 44}}) + + # Should only receive one orig_config and then the changed_config + async with asyncio.timeout(1): + result = await receiver.receive() + assert result == SimpleConfig(name="test3", value=44) + + # There should be no more messages + with pytest.raises(asyncio.TimeoutError): + async with asyncio.timeout(0.1): + await receiver.receive() + + +async def test_skip_unchanged_config(mocker: pytest_mock.MockerFixture) -> None: + """Test that unchanged configurations are skipped when skip_unchanged is True.""" + mocker.patch("frequenz.sdk.config._manager.ConfigManagingActor") + config_manager = ConfigManager([pathlib.Path("dummy.toml")]) + sender = config_manager.config_channel.new_sender() + receiver = config_manager.new_receiver( + "test", + SimpleConfig, + skip_unchanged=True, + ) + + # A first config should be received + orig_config = {"test": {"name": "test1", "value": 42}} + await sender.send(orig_config) + async with asyncio.timeout(1): + result = await receiver.receive() + assert result == SimpleConfig(name="test1", value=42) + + # An unchanged config should be skipped (no message received) + await sender.send(orig_config) + with pytest.raises(asyncio.TimeoutError): + async with asyncio.timeout(0.1): + await receiver.receive() + + # A changed config should be received + changed_config = {"test": {"name": "test2", "value": 43}} + await sender.send(changed_config) + async with asyncio.timeout(1): + result = await receiver.receive() + assert result == SimpleConfig(name="test2", value=43) + + # There should be no more messages + with pytest.raises(asyncio.TimeoutError): + async with asyncio.timeout(0.1): + await receiver.receive() + + +async def test_wait_for_first(mocker: pytest_mock.MockerFixture) -> None: + """Test wait_for_first function.""" + mocker.patch("frequenz.sdk.config._manager.ConfigManagingActor") + config_manager = ConfigManager([pathlib.Path("dummy.toml")]) + + receiver = config_manager.new_receiver( + "test", + SimpleConfig, + ) + + async with asyncio.timeout(0.2): + with pytest.raises(asyncio.TimeoutError): + await wait_for_first(receiver, timeout=timedelta(seconds=0.1)) + + # Test successful wait + await config_manager.config_channel.new_sender().send( + {"test": {"name": "test1", "value": 42}} + ) + async with asyncio.timeout(0.2): + result = await wait_for_first(receiver, timeout=timedelta(seconds=0.1)) + assert result == SimpleConfig(name="test1", value=42) + + +def test_unknown_include_not_supported() -> None: + """Test that unknown marshmallow load kwargs are not supported.""" + with pytest.raises(ValueError): + ConfigManager([pathlib.Path("dummy.toml")]).new_receiver( + "test", + SimpleConfig, + marshmallow_load_kwargs={"unknown": marshmallow.INCLUDE}, + ) + + +@pytest.mark.integration +class TestConfigManagerIntegration: + """Integration tests for ConfigManager.""" + + @pytest.fixture + def config_file(self, tmp_path: pathlib.Path) -> pathlib.Path: + """Create a temporary config file for testing.""" + config_file = tmp_path / "config.toml" + config_file.write_text( + """ + [test] + name = "test1" + value = 42 + + [logging.loggers.test] + level = "DEBUG" + """ + ) + return config_file + + async def test_full_config_flow(self, config_file: pathlib.Path) -> None: + """Test the complete flow of configuration management.""" + async with ( + # Disabling force_polling is a hack because of a bug in watchfiles not + # detecting sub-second changes when using polling. + ConfigManager([config_file], force_polling=False) as config_manager, + asyncio.timeout(1), + ): + receiver = config_manager.new_receiver("test", SimpleConfig) + first_config = await wait_for_first(receiver) + assert first_config == SimpleConfig(name="test1", value=42) + assert logging.getLogger("test").level == logging.DEBUG + + # Update config file + config_file.write_text( + """ + [test] + name = "test2" + value = 43 + + [logging.loggers.test] + level = "INFO" + """ + ) + + # Check updated config + config = await receiver.receive() + assert config == SimpleConfig(name="test2", value=43) + + # Check updated logging config + assert logging.getLogger("test").level == logging.INFO + + async def test_full_config_flow_without_logging( + self, config_file: pathlib.Path + ) -> None: + """Test the complete flow of configuration management without logging.""" + logging.getLogger("test").setLevel(logging.WARNING) + async with ( + # Disabling force_polling is a hack because of a bug in watchfiles not + # detecting sub-second changes when using polling. + ConfigManager( + [config_file], logging_config_key=None, force_polling=False + ) as config_manager, + asyncio.timeout(1), + ): + receiver = config_manager.new_receiver("test", SimpleConfig) + first_config = await wait_for_first(receiver) + assert first_config == SimpleConfig(name="test1", value=42) + assert logging.getLogger("test").level == logging.WARNING + + # Update config file + config_file.write_text( + """ + [test] + name = "test2" + value = 43 + + [logging.loggers.test] + level = "DEBUG" + """ + ) + + # Check updated config + config = await receiver.receive() + assert config == SimpleConfig(name="test2", value=43) + + # Check updated logging config + assert logging.getLogger("test").level == logging.WARNING + + +@dataclass(frozen=True) +class GetKeyTestCase: + """Test case for _get_key function.""" + + title: str + config: dict[str, Any] + key: str | Sequence[str] + expected_result: Mapping[str, Any] | None | type[InvalidValueForKeyError] + expected_error_key: list[str] | None = None + expected_error_value: Any | None = None + + +_get_key_test_cases = [ + # Simple string key tests + GetKeyTestCase( + title="Simple string key - exists", + config={"a": {"b": 1}}, + key="a", + expected_result={"b": 1}, + ), + GetKeyTestCase( + title="Simple string key - doesn't exist", + config={"a": {"b": 1}}, + key="x", + expected_result=None, + ), + GetKeyTestCase( + title="Simple string key - invalid value type", + config={"a": 42}, + key="a", + expected_result=InvalidValueForKeyError, + expected_error_key=["a"], + expected_error_value=42, + ), + # Sequence key tests + GetKeyTestCase( + title="Sequence key - all exist", + config={"a": {"b": {"c": {"d": 1}}}}, + key=["a", "b", "c"], + expected_result={"d": 1}, + ), + GetKeyTestCase( + title="Sequence key - middle doesn't exist", + config={"a": {"b": {"c": 1}}}, + key=["a", "x", "c"], + expected_result=None, + ), + GetKeyTestCase( + title="Sequence key - invalid value in middle", + config={"a": {"b": 42, "c": 1}}, + key=["a", "b", "c"], + expected_result=InvalidValueForKeyError, + expected_error_key=["a", "b"], + expected_error_value=42, + ), + GetKeyTestCase( + title="Empty sequence key", + config={"a": 1}, + key=[], + expected_result={"a": 1}, + ), + # Empty string tests + GetKeyTestCase( + title="Empty string key", + config={"": {"a": 1}}, + key="", + expected_result={"a": 1}, + ), + GetKeyTestCase( + title="Empty string in sequence", + config={"": {"": {"a": 1}}}, + key=["", ""], + expected_result={"a": 1}, + ), + # None value tests + GetKeyTestCase( + title="Value is None", + config={"a": None}, + key="a", + expected_result=None, + ), + GetKeyTestCase( + title="Nested None value", + config={"a": {"b": None}}, + key=["a", "b", "c"], + expected_result=None, + ), + # Special string key tests to verify string handling + GetKeyTestCase( + title="String that looks like a sequence", + config={"key": {"e": 1}}, + key="key", + expected_result={"e": 1}, + ), + GetKeyTestCase( + title="String with special characters", + config={"a.b": {"c": 1}}, + key="a.b", + expected_result={"c": 1}, + ), + # Nested mapping tests + GetKeyTestCase( + title="Deeply nested valid path", + config={"a": {"b": {"c": {"d": {"e": {"f": 1}}}}}}, + key=["a", "b", "c", "d", "e"], + expected_result={"f": 1}, + ), + GetKeyTestCase( + title="Mixed type nested invalid", + config={"a": {"b": [1, 2, 3]}}, + key=["a", "b"], + expected_result=InvalidValueForKeyError, + expected_error_key=["a", "b"], + expected_error_value=[1, 2, 3], + ), +] + + +@pytest.mark.parametrize("test_case", _get_key_test_cases, ids=lambda tc: tc.title) +def test_get_key(test_case: GetKeyTestCase) -> None: + """Test the _get_key function with various inputs. + + Args: + test_case: The test case to run. + """ + if test_case.expected_result is InvalidValueForKeyError: + with pytest.raises(InvalidValueForKeyError) as exc_info: + _get_key(test_case.config, test_case.key) + + # Verify the error details + assert exc_info.value.key == test_case.expected_error_key + assert exc_info.value.value == test_case.expected_error_value + else: + result = _get_key(test_case.config, test_case.key) + assert result == test_case.expected_result From d02cd1a3cb5949e671fb1f50ef339a134f731e38 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 22 Nov 2024 13:29:39 +0100 Subject: [PATCH 17/18] Add a config user guide and improve the documentation Add module-level documentation to the `frequenz.sdk.config` module and include it in the generated documentation's user guide. Signed-off-by: Leandro Lucarella --- docs/user-guide/config.md | 9 + src/frequenz/sdk/config/__init__.py | 425 +++++++++++++++++++++++++++- src/frequenz/sdk/config/_manager.py | 84 ++---- 3 files changed, 451 insertions(+), 67 deletions(-) create mode 100644 docs/user-guide/config.md diff --git a/docs/user-guide/config.md b/docs/user-guide/config.md new file mode 100644 index 000000000..be4e42f77 --- /dev/null +++ b/docs/user-guide/config.md @@ -0,0 +1,9 @@ +# Configuration + +::: frequenz.sdk.config + options: + members: [] + show_bases: false + show_root_heading: false + show_root_toc_entry: false + show_source: false diff --git a/src/frequenz/sdk/config/__init__.py b/src/frequenz/sdk/config/__init__.py index 8355ac7f7..bf564fa26 100644 --- a/src/frequenz/sdk/config/__init__.py +++ b/src/frequenz/sdk/config/__init__.py @@ -1,7 +1,430 @@ # License: MIT # Copyright © 2024 Frequenz Energy-as-a-Service GmbH -"""Configuration management.""" +"""Configuration management. + +# Overview + +To provide dynamic configurations to an application, you can use the +[`ConfigManager`][frequenz.sdk.config.ConfigManager] class. This class provides +a convenient interface to manage configurations from multiple config files and receive +updates when the configurations change. Users can create a receiver to receive +configurations from the manager. + +# Setup + +To use the `ConfigManager`, you need to create an instance of it and pass the +paths to the configuration files. The configuration files must be in the TOML +format. + +When specifying multiple files order matters, as the configuration will be read and +updated in the order of the paths, so the last path will override the configuration set +by the previous paths. Dict keys will be merged recursively, but other objects (like +lists) will be replaced by the value in the last path. + +```python +from frequenz.sdk.config import ConfigManager + +async with ConfigManager(["base-config.toml", "overrides.toml"]) as config_manager: + ... +``` + +# Logging + +The `ConfigManager` can also instantiate +a [`LoggingConfigUpdatingActor`][frequenz.sdk.config.LoggingConfigUpdatingActor] to +monitor logging configurations. This actor will listen for logging configuration changes +and update the logging configuration accordingly. + +This feature is enabled by default using the key `logging` in the configuration file. To +disable it you can pass `logging_config_key=None` to the `ConfigManager`. + +# Receiving configurations + +To receive configurations, you can create a receiver using the [`new_receiver()`][ +frequenz.sdk.config.ConfigManager.new_receiver] method. The receiver will receive +configurations from the manager for a particular key, and validate and load the +configurations to a dataclass using [`marshmallow_dataclass`][]. + +If the key is a sequence of strings, it will be treated as a nested key and the +receiver will receive the configuration under the nested key. For example +`["key", "subkey"]` will get only `config["key"]["subkey"]`. + +Besides a configuration instance, the receiver can also receive exceptions if there are +errors loading the configuration (typically +a [`ValidationError`][marshmallow.ValidationError]), or `None` if there is no +configuration for the key. + +The value under `key` must be another mapping, otherwise +a [`InvalidValueForKeyError`][frequenz.sdk.config.InvalidValueForKeyError] instance will +be sent to the receiver. + +If there were any errors loading the configuration, the error will be logged too. + +```python +from dataclasses import dataclass +from frequenz.sdk.config import ConfigManager + +@dataclass(frozen=True, kw_only=True) +class AppConfig: + test: int + +async with ConfigManager("config.toml") as config_manager: + receiver = config_manager.new_receiver("app", AppConfig) + app_config = await receiver.receive() + match app_config: + case AppConfig(test=42): + print("App configured with 42") + case Exception() as error: + print(f"Error loading configuration: {error}") + case None: + print("There is no configuration for the app key") +``` + +## Validation and loading + +The configuration class used to create the configuration instance is expected to be +a [`dataclasses.dataclass`][], which is used to create a [`marshmallow.Schema`][] via +the [`marshmallow_dataclass.class_schema`][] function. + +This means you can customize the schema derived from the configuration +dataclass using [`marshmallow_dataclass`][] to specify extra validation and +options via field metadata. + +Customization can also be done via a `base_schema`. By default +[`BaseConfigSchema`][frequenz.sdk.config.BaseConfigSchema] is used to provide support +for some extra commonly used fields (like [quantities][frequenz.quantities]). + +```python +import marshmallow.validate +from dataclasses import dataclass, field + +@dataclass(frozen=True, kw_only=True) +class Config: + test: int = field( + metadata={"validate": marshmallow.validate.Range(min=0)}, + ) +``` + +Additional arguments can be passed to [`marshmallow.Schema.load`][] using +the `marshmallow_load_kwargs` keyword arguments. + +If unspecified, the `marshmallow_load_kwargs` will have the `unknown` key set to +[`marshmallow.EXCLUDE`][] (instead of the normal [`marshmallow.RAISE`][] +default). + +But when [`marshmallow.EXCLUDE`][] is used, a warning will be logged if there are extra +fields in the configuration that are excluded. This is useful, for example, to catch +typos in the configuration file. + +## Skipping superfluous updates + +If there is a burst of configuration updates, the receiver will only receive the +last configuration, older configurations will be ignored. + +If `skip_unchanged` is set to `True`, then a configuration that didn't change +compared to the last one received will be ignored and not sent to the receiver. +The comparison is done using the *raw* `dict` to determine if the configuration +has changed. + +## Error handling + +The value under `key` must be another mapping, otherwise an error +will be logged and a [`frequenz.sdk.config.InvalidValueForKeyError`][] instance +will be sent to the receiver. + +Configurations that don't pass the validation will be logged as an error and +the [`ValidationError`][marshmallow.ValidationError] sent to the receiver. + +Any other unexpected error raised during the configuration loading will be +logged as an error and the error instance sent to the receiver. + +## Further customization + +If you have special needs for receiving the configurations (for example validating using +`marshmallow` doesn't fit your needs), you can create a custom receiver using +[`config_channel.new_receiver()`][frequenz.sdk.config.ConfigManager.config_channel] +directly. Please bear in mind that this provides a low-level access to the whole config +in the file as a raw Python mapping. + +# Recommended usage + +Actors that need to be reconfigured should take a configuration manager and a key to +receive configurations updates, and instantiate the new receiver themselves. This allows +actors to have full control over how the configuration is loaded (for example providing +a custom base schema or marshmallow options). + +Passing the key explicitly too allows application to structure the configuration in +whatever way is most convenient for the application. + +Actors can use the [`wait_for_first()`][frequenz.sdk.config.wait_for_first] function to +wait for the first configuration to be received, and cache the configuration for later +use and in case the actor is restarted. If the configuration is not received after some +timeout, a [`asyncio.TimeoutError`][] will be raised (and if uncaught, the actor will +be automatically restarted after some delay). + +Example: Actor that can run without a configuration (using a default configuration) + ```python title="actor.py" hl_lines="18 34 42 62 64" + import dataclasses + import logging + from collections.abc import Sequence + from datetime import timedelta + from typing import assert_never + + from frequenz.channels import select, selected_from + from frequenz.channels.event import Event + + from frequenz.sdk.actor import Actor + from frequenz.sdk.config import ConfigManager, wait_for_first + + _logger = logging.getLogger(__name__) + + @dataclasses.dataclass(frozen=True, kw_only=True) + class MyActorConfig: + some_config: timedelta = dataclasses.field( + default=timedelta(seconds=42), # (1)! + metadata={"metadata": {"description": "Some optional configuration"}}, + ) + + class MyActor(Actor): + def __init__( + self, + config_manager: ConfigManager, + /, + *, + config_key: str | Sequence[str], + name: str | None = None, + ) -> None: + super().__init__(name=name) + self._config_manager = config_manager + self._config_key = config_key + self._config: MyActorConfig = MyActorConfig() # (2)! + + async def _run(self) -> None: + config_receiver = self._config_manager.new_receiver( + self._config_key, MyActorConfig + ) + self._update_config( + await wait_for_first( + config_receiver, receiver_name=str(self), allow_none=True # (3)! + ) + ) + + other_receiver = Event() + + async for selected in select(config_receiver, other_receiver): + if selected_from(selected, config_receiver): + self._update_config(selected.message) + elif selected_from(selected, other_receiver): + # Do something else + ... + + def _update_config(self, config_update: MyActorConfig | Exception | None) -> None: + match config_update: + case MyActorConfig() as config: + _logger.info("New configuration received, updating.") + self._reconfigure(config) + case None: + _logger.info("Configuration was unset, resetting to the default") + self._reconfigure(MyActorConfig()) # (4)! + case Exception(): + _logger.info( # (5)! + "New configuration has errors, keeping the old configuration." + ) + case unexpected: + assert_never(unexpected) + + def _reconfigure(self, config: MyActorConfig) -> None: + self._config = config + # Do something with the new configuration + ``` + + 1. This is different when the actor requires a configuration to run. Here, the + config has a default value. + 2. This is different when the actor requires a configuration to run. Here, the actor + can just instantiate a default configuration. + 3. This is different when the actor requires a configuration to run. Here, the actor + can accept a `None` configuration. + 4. This is different when the actor requires a configuration to run. Here, the actor + can reset to a default configuration. + 5. There is no need to log the error itself, the configuration manager will log it + automatically. + +Example: Actor that requires a configuration to run + ```python title="actor.py" hl_lines="17 33 40 58 60" + import dataclasses + import logging + from collections.abc import Sequence + from datetime import timedelta + from typing import assert_never + + from frequenz.channels import select, selected_from + from frequenz.channels.event import Event + + from frequenz.sdk.actor import Actor + from frequenz.sdk.config import ConfigManager, wait_for_first + + _logger = logging.getLogger(__name__) + + @dataclasses.dataclass(frozen=True, kw_only=True) + class MyActorConfig: + some_config: timedelta = dataclasses.field( # (1)! + metadata={"metadata": {"description": "Some required configuration"}}, + ) + + class MyActor(Actor): + def __init__( + self, + config_manager: ConfigManager, + /, + *, + config_key: str | Sequence[str], + name: str | None = None, + ) -> None: + super().__init__(name=name) + self._config_manager = config_manager + self._config_key = config_key + self._config: MyActorConfig # (2)! + + async def _run(self) -> None: + config_receiver = self._config_manager.new_receiver( + self._config_key, MyActorConfig + ) + self._update_config( + await wait_for_first(config_receiver, receiver_name=str(self)) # (3)! + ) + + other_receiver = Event() + + async for selected in select(config_receiver, other_receiver): + if selected_from(selected, config_receiver): + self._update_config(selected.message) + elif selected_from(selected, other_receiver): + # Do something else + ... + + def _update_config(self, config_update: MyActorConfig | Exception | None) -> None: + match config_update: + case MyActorConfig() as config: + _logger.info("New configuration received, updating.") + self._reconfigure(config) + case None: + _logger.info("Configuration was unset, keeping the old configuration.") # (4)! + case Exception(): + _logger.info( # (5)! + "New configuration has errors, keeping the old configuration." + ) + case unexpected: + assert_never(unexpected) + + def _reconfigure(self, config: MyActorConfig) -> None: + self._config = config + # Do something with the new configuration + ``` + + 1. This is different when the actor can use a default configuration. Here, the + field is required, so there is no default configuration possible. + 2. This is different when the actor can use a default configuration. Here, the + assignment of the configuration is delayed to the `_run()` method. + 3. This is different when the actor can use a default configuration. Here, the actor + doesn't accept `None` as a valid configuration as it can't create a default + configuration. + 4. This is different when the actor can use a default configuration. Here, the actor + doesn't accept `None` as a valid configuration as it can't create a default + configuration, so it needs to keep the old configuration. + 5. There is no need to log the error itself, the configuration manager will log it + automatically. + + +Example: Application + The pattern used by the application is very similar to the one used by actors. In + this case the application requires a configuration to run, but if it could also use + a default configuration, the changes would be the same as in the actor examples. + + ```python title="app.py" hl_lines="14" + import asyncio + import dataclasses + import logging + import pathlib + from collections.abc import Sequence + from datetime import timedelta + from typing import Sequence, assert_never + + from frequenz.sdk.actor import Actor + from frequenz.sdk.config import ConfigManager, wait_for_first + + _logger = logging.getLogger(__name__) + + class MyActor(Actor): # (1)! + def __init__( + self, config_manager: ConfigManager, /, *, config_key: str | Sequence[str] + ) -> None: + super().__init__() + self._config_manager = config_manager + self._config_key = config_key + async def _run(self) -> None: ... + + @dataclasses.dataclass(frozen=True, kw_only=True) + class AppConfig: + enable_actor: bool = dataclasses.field( + metadata={"metadata": {"description": "Whether to enable the actor"}}, + ) + + class App: + def __init__(self, *, config_paths: Sequence[pathlib.Path]): + self._config_manager = ConfigManager(config_paths) + self._config_receiver = self._config_manager.new_receiver("app", AppConfig) + self._actor = MyActor(self._config_manager, config_key="actor") + + async def _update_config(self, config_update: AppConfig | Exception | None) -> None: + match config_update: + case AppConfig() as config: + _logger.info("New configuration received, updating.") + await self._reconfigure(config) + case None: + _logger.info("Configuration was unset, keeping the old configuration.") + case Exception(): + _logger.info("New configuration has errors, keeping the old configuration.") + case unexpected: + assert_never(unexpected) + + async def _reconfigure(self, config: AppConfig) -> None: + if config.enable_actor: + self._actor.start() + else: + await self._actor.stop() + + async def run(self) -> None: + _logger.info("Starting App...") + + async with self._config_manager: + await self._update_config( + await wait_for_first(self._config_receiver, receiver_name="app") + ) + + _logger.info("Waiting for configuration updates...") + async for config_update in self._config_receiver: + await self._reconfigure(config_update) + + if __name__ == "__main__": + asyncio.run(App(config_paths="config.toml").run()) + ``` + + 1. Look for the actor examples for a proper implementation of the actor. + + Example configuration file: + + ```toml title="config.toml" + [app] + enable_actor = true + + [actor] + some_config = 10 + + [logging.root_logger] + level = "DEBUG" + ``` +""" from ._base_schema import BaseConfigSchema from ._logging_actor import LoggerConfig, LoggingConfig, LoggingConfigUpdatingActor diff --git a/src/frequenz/sdk/config/_manager.py b/src/frequenz/sdk/config/_manager.py index eb3552984..61d969915 100644 --- a/src/frequenz/sdk/config/_manager.py +++ b/src/frequenz/sdk/config/_manager.py @@ -49,7 +49,10 @@ class ConfigManager(BackgroundService): """A manager for configuration files. This class reads configuration files and sends the configuration to the receivers, - providing optional configuration key filtering and schema validation. + providing configuration key filtering and value validation. + + For a more in-depth introduction and examples, please read the [module + documentation][frequenz.sdk.config]. """ def __init__( # pylint: disable=too-many-arguments @@ -85,7 +88,12 @@ def __init__( # pylint: disable=too-many-arguments self.config_channel: Final[Broadcast[Mapping[str, Any]]] = Broadcast( name=f"{self}_config", resend_latest=True ) - """The broadcast channel for the configuration.""" + """The channel used for sending configuration updates (resends the latest value). + + This is the channel used to communicate with the + [`ConfigManagingActor`][frequenz.sdk.config.ConfigManager.config_actor] and will + receive the complete raw configuration as a mapping. + """ self.config_actor: Final[ConfigManagingActor] = ConfigManagingActor( config_paths, @@ -94,7 +102,7 @@ def __init__( # pylint: disable=too-many-arguments force_polling=force_polling, polling_interval=polling_interval, ) - """The actor that manages the configuration.""" + """The actor that manages the configuration for this manager.""" # pylint: disable-next=import-outside-toplevel,cyclic-import from ._logging_actor import LoggingConfigUpdatingActor @@ -106,6 +114,7 @@ def __init__( # pylint: disable=too-many-arguments self, config_key=logging_config_key, name=self.name ) ) + """The actor that manages the logging configuration for this manager.""" @override def start(self) -> None: @@ -196,68 +205,8 @@ def new_receiver( # pylint: disable=too-many-arguments receiver, you can create a receiver directly using [`config_channel.new_receiver()`][frequenz.sdk.config.ConfigManager.config_channel]. - ### Filtering - - Only the configuration under the `key` will be received by the receiver. If the - `key` is not found in the configuration, the receiver will receive `None`. - - If the key is a sequence of strings, it will be treated as a nested key and the - receiver will receive the configuration under the nested key. For example - `["key", "subkey"]` will get only `config["key"]["subkey"]`. - - The value under `key` must be another mapping, otherwise an error - will be logged and a [`frequenz.sdk.config.InvalidValueForKeyError`][] instance - will be sent to the receiver. - - ### Schema validation - - The raw configuration received as a `Mapping` will be validated and loaded to - as a `config_class`. The `config_class` class is expected to be - a [`dataclasses.dataclass`][], which is used to create - a [`marshmallow.Schema`][] via the [`marshmallow_dataclass.class_schema`][] - function. - - This means you can customize the schema derived from the configuration - dataclass using [`marshmallow_dataclass`][] to specify extra validation and - options via field metadata. - - Additional arguments can be passed to [`marshmallow.Schema.load`][] using - the `marshmallow_load_kwargs` keyword arguments. - - If unspecified, the `marshmallow_load_kwargs` will have the `unknown` key set to - [`marshmallow.EXCLUDE`][] (instead of the normal [`marshmallow.RAISE`][] - default). - - But when [`marshmallow.EXCLUDE`][] is used, a warning will still be logged if - there are extra fields in the configuration that are excluded. This is useful, - for example, to catch typos in the configuration file. - - ### Skipping superfluous updates - - If there is a burst of configuration updates, the receiver will only receive the - last configuration, older configurations will be ignored. - - If `skip_unchanged` is set to `True`, then a configuration that didn't change - compared to the last one received will be ignored and not sent to the receiver. - The comparison is done using the *raw* `dict` to determine if the configuration - has changed. - - ### Error handling - - The value under `key` must be another mapping, otherwise an error - will be logged and a [`frequenz.sdk.config.InvalidValueForKeyError`][] instance - will be sent to the receiver. - - Configurations that don't pass the validation will be logged as an error and - the [`ValidationError`][marshmallow.ValidationError] sent to the receiver. - - Any other unexpected error raised during the configuration loading will be - logged as an error and the error instance sent to the receiver. - - Example: - ```python - # TODO: Add Example - ``` + For a more in-depth introduction and examples, please read the [module + documentation][frequenz.sdk.config]. Args: key: The configuration key to be read by the receiver. If a sequence of @@ -345,7 +294,10 @@ async def wait_for_first( allow_none: bool = False, timeout: timedelta = timedelta(minutes=1), ) -> DataclassT | None: - """Receive the first configuration. + """Wait for and receive the the first configuration. + + For a more in-depth introduction and examples, please read the [module + documentation][frequenz.sdk.config]. Args: receiver: The receiver to receive the first configuration from. From 4c87e6a2097c68d1eb235c0bc40dc7879d4a0de0 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 20 Dec 2024 13:09:53 +0100 Subject: [PATCH 18/18] Update release notes Signed-off-by: Leandro Lucarella --- RELEASE_NOTES.md | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 7d2ad13fa..46fd449ae 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -2,7 +2,7 @@ ## Summary - +This release includes a new `ConfigManager` class to simplify managing the configuration, and ships other improvements and fixes to the config system in general. ## Upgrading @@ -33,12 +33,30 @@ + The `base_schema` argument is now keyword-only. + The arguments forwarded to `marshmallow.Schema.load()` now must be passed explicitly via the `marshmallow_load_kwargs` argument, as a `dict`, to improve the type-checking. + * `ConfigManagingActor`: Raise a `ValueError` if the `config_files` argument an empty sequence. + ## New Features -- `LoggingConfigUpdatingActor` +- `frequenz.sdk.config` + + - Logging was improved in general. + + - Added documentation and user guide. + + - `LoggingConfigUpdatingActor` - * Added a new `name` argument to the constructor to be able to override the actor's name. + * Added a new `name` argument to the constructor to be able to override the actor's name. + + - `ConfigManager`: Added a class to simplify managing the configuration. It takes care of instantiating the config actors and provides a convenient method for creating receivers with a lot of common functionality. + + - `BaseConfigSchema`: Added a `marshmallow` base `Schema` that includes custom fields for `frequenz-quantities`. In the futute more commonly used fields might be added. + + - `wait_for_first()`: Added a function to make it easy to wait for the first configuration to be received with a timeout. + + - `ConfigManagingActor`: Allow passing a single configuration file. ## Bug Fixes - Fix a bug in `BackgroundService` where it won't try to `self.cancel()` and `await self.wait()` if there are no internal tasks. This prevented to properly implement custom stop logic without having to redefine the `stop()` method too. + +- Fix a bug where if a string was passed to the `ConfigManagingActor` it would be interpreted as a sequence of 1 character strings.