From 62afee8d2a5eb3b87cd45b4a02de51c521b5a072 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 25 Apr 2025 10:24:49 +0200 Subject: [PATCH 1/2] Fix mkdocstrings path config This configuration was at the wrong level. Signed-off-by: Leandro Lucarella --- mkdocs.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mkdocs.yml b/mkdocs.yml index bb59da720..ad7dba795 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -101,8 +101,8 @@ plugins: default_handler: python handlers: python: + paths: ["src"] options: - paths: ["src"] docstring_section_style: spacy inherited_members: true merge_init_into_class: false From c6cf3656d37f641499b08de1b5bc083036d206fe Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 25 Apr 2025 10:51:27 +0200 Subject: [PATCH 2/2] Update the logging config actor configuration format The logging configuration was changed to make it compatible with the configuration generated by the UI, which doesn't quote sub-key properly. To do this, the logger name needs to be specified explicitly now (the configuration key was used as the name before). Signed-off-by: Leandro Lucarella --- RELEASE_NOTES.md | 31 +++++++++++++++ src/frequenz/sdk/config/__init__.py | 8 +++- src/frequenz/sdk/config/_logging_actor.py | 46 ++++++++++++++++------- tests/config/test_logging_actor.py | 23 +++++++----- tests/config/test_manager.py | 3 ++ 5 files changed, 86 insertions(+), 25 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index cf471e1dd..3cc8651fe 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -8,6 +8,37 @@ * Includes a major update of the numpy dependency to v2.x. +* The logging configuration was changed, and now the logger name needs to be specified explicitly (the configuration key was used as the name before). This is to be compatible with configuration generated by the UI, which doesn't quote sub-key properly. + + - Before: + + ```toml + [logging.loggers."frequenz.sdk.config"] + level = "DEBUG" + ``` + + - Now: + + ```toml + [logging.loggers.frequenz_config] + name = "frequenz.sdk.config" + level = "DEBUG" + ``` + +* The logging configuration now uses a separate class for the root logger configuration: `RootLoggerConfig`. + + - Before: + + ```py + LoggingConfig(root_logger=LoggerConfig(level="ERROR")) + ``` + + - Now: + + ```py + LoggingConfig(root_logger=RootLoggerConfig(level="ERROR")) + ``` + ## New Features diff --git a/src/frequenz/sdk/config/__init__.py b/src/frequenz/sdk/config/__init__.py index 461bc7e8a..199cbedc1 100644 --- a/src/frequenz/sdk/config/__init__.py +++ b/src/frequenz/sdk/config/__init__.py @@ -424,7 +424,12 @@ async def run(self) -> None: """ from ._base_schema import BaseConfigSchema -from ._logging_actor import LoggerConfig, LoggingConfig, LoggingConfigUpdatingActor +from ._logging_actor import ( + LoggerConfig, + LoggingConfig, + LoggingConfigUpdatingActor, + RootLoggerConfig, +) from ._manager import ConfigManager, InvalidValueForKeyError, wait_for_first from ._managing_actor import ConfigManagingActor from ._util import load_config @@ -437,6 +442,7 @@ async def run(self) -> None: "LoggerConfig", "LoggingConfig", "LoggingConfigUpdatingActor", + "RootLoggerConfig", "load_config", "wait_for_first", ] diff --git a/src/frequenz/sdk/config/_logging_actor.py b/src/frequenz/sdk/config/_logging_actor.py index 739e0be07..c45a3f380 100644 --- a/src/frequenz/sdk/config/_logging_actor.py +++ b/src/frequenz/sdk/config/_logging_actor.py @@ -25,8 +25,8 @@ @dataclass(frozen=True, kw_only=True) -class LoggerConfig: - """A configuration for a logger.""" +class RootLoggerConfig: + """A configuration for the root logger.""" level: LogLevel = field( default="NOTSET", @@ -36,15 +36,29 @@ class LoggerConfig: }, }, ) - """The log level for the logger.""" + """The log level for the root logger.""" + + +@dataclass(frozen=True, kw_only=True) +class LoggerConfig(RootLoggerConfig): + """A configuration for a logger.""" + + name: str = field( + metadata={ + "metadata": { + "description": "The name of the logger that will be affected by this " + "configuration." + }, + }, + ) @dataclass(frozen=True, kw_only=True) class LoggingConfig: """A configuration for the logging system.""" - root_logger: LoggerConfig = field( - default_factory=lambda: LoggerConfig(level="INFO"), + root_logger: RootLoggerConfig = field( + default_factory=lambda: RootLoggerConfig(level="INFO"), metadata={ "metadata": { "description": "Default default configuration for all loggers.", @@ -73,10 +87,12 @@ class LoggingConfigUpdatingActor(Actor): [logging.root_logger] level = "INFO" - [logging.loggers."frequenz.sdk.actor.power_distributing"] + [logging.loggers.power_dist] + name = "frequenz.sdk.actor.power_distributing" level = "DEBUG" - [logging.loggers."frequenz.channels"] + [logging.loggers.chan] + name = "frequenz.channels" level = "DEBUG" ``` @@ -184,10 +200,12 @@ def _reconfigure(self, config_update: LoggingConfig | Exception | None) -> None: def _update_logging(self, config: LoggingConfig) -> None: """Configure the logging level.""" # If the logger is not in the new config, set it to NOTSET - loggers_to_unset = self._current_config.loggers.keys() - config.loggers.keys() - for logger_id in loggers_to_unset: - _logger.debug("Unsetting log level for logger '%s'", logger_id) - logging.getLogger(logger_id).setLevel(logging.NOTSET) + old_names = {old.name for old in self._current_config.loggers.values()} + new_names = {new.name for new in config.loggers.values()} + loggers_to_unset = old_names - new_names + for logger_name in loggers_to_unset: + _logger.debug("Unsetting log level for logger '%s'", logger_name) + logging.getLogger(logger_name).setLevel(logging.NOTSET) self._current_config = config _logger.info( @@ -196,12 +214,12 @@ def _update_logging(self, config: LoggingConfig) -> None: logging.getLogger().setLevel(self._current_config.root_logger.level) # For each logger in the new config, set the log level - for logger_id, logger_config in self._current_config.loggers.items(): + for logger_config in self._current_config.loggers.values(): _logger.info( "Setting log level for logger '%s' to '%s'", - logger_id, + logger_config.name, logger_config.level, ) - logging.getLogger(logger_id).setLevel(logger_config.level) + logging.getLogger(logger_config.name).setLevel(logger_config.level) _logger.info("Logging config update completed.") diff --git a/tests/config/test_logging_actor.py b/tests/config/test_logging_actor.py index c06aa07dc..1ee2d92ee 100644 --- a/tests/config/test_logging_actor.py +++ b/tests/config/test_logging_actor.py @@ -16,6 +16,7 @@ LoggerConfig, LoggingConfig, LoggingConfigUpdatingActor, + RootLoggerConfig, load_config, ) @@ -25,15 +26,15 @@ def test_logging_config() -> None: config_raw = { "root_logger": {"level": "DEBUG"}, "loggers": { - "frequenz.sdk.actor": {"level": "INFO"}, - "frequenz.sdk.timeseries": {"level": "ERROR"}, + "actor": {"name": "frequenz.sdk.actor", "level": "INFO"}, + "timeseries": {"name": "frequenz.sdk.timeseries", "level": "ERROR"}, }, } config = LoggingConfig( - root_logger=LoggerConfig(level="DEBUG"), + root_logger=RootLoggerConfig(level="DEBUG"), loggers={ - "frequenz.sdk.actor": LoggerConfig(level="INFO"), - "frequenz.sdk.timeseries": LoggerConfig(level="ERROR"), + "actor": LoggerConfig(name="frequenz.sdk.actor", level="INFO"), + "timeseries": LoggerConfig(name="frequenz.sdk.timeseries", level="ERROR"), }, ) @@ -92,10 +93,12 @@ async def test_logging_config_updating_actor( # Send first config expected_config = LoggingConfig( - root_logger=LoggerConfig(level="ERROR"), + root_logger=RootLoggerConfig(level="ERROR"), loggers={ - "frequenz.sdk.actor": LoggerConfig(level="DEBUG"), - "frequenz.sdk.timeseries": LoggerConfig(level="ERROR"), + "actor": LoggerConfig(name="frequenz.sdk.actor", level="DEBUG"), + "timeseries": LoggerConfig( + name="frequenz.sdk.timeseries", level="ERROR" + ), }, ) await mock_config_manager.config_channel.new_sender().send(expected_config) @@ -121,9 +124,9 @@ async def test_logging_config_updating_actor( # Update config expected_config = LoggingConfig( - root_logger=LoggerConfig(level="WARNING"), + root_logger=RootLoggerConfig(level="WARNING"), loggers={ - "frequenz.sdk.actor": LoggerConfig(level="INFO"), + "actor": LoggerConfig(name="frequenz.sdk.actor", level="INFO"), }, ) await mock_config_manager.config_channel.new_sender().send(expected_config) diff --git a/tests/config/test_manager.py b/tests/config/test_manager.py index b36154758..131e973fa 100644 --- a/tests/config/test_manager.py +++ b/tests/config/test_manager.py @@ -250,6 +250,7 @@ def config_file(self, tmp_path: pathlib.Path) -> pathlib.Path: value = 42 [logging.loggers.test] + name = "test" level = "DEBUG" """ ) @@ -276,6 +277,7 @@ async def test_full_config_flow(self, config_file: pathlib.Path) -> None: value = 43 [logging.loggers.test] + name = "test" level = "INFO" """ ) @@ -313,6 +315,7 @@ async def test_full_config_flow_without_logging( value = 43 [logging.loggers.test] + name = "test" level = "DEBUG" """ )