Skip to content

Commit b0a656e

Browse files
authored
Improve the logging config actor (frequenz-floss#1127)
- **Make all arguments keyword-only** - **Add a `name` argument to `LoggingConfigUpdatingActor`** - **Sort arguments in `LoggingConfigUpdatingActor` constructor** - **Initialize parent class after all attributes were initialized** - **Add missing docstring for `LogLevel`** - **Remove the `LoggingConfig.load` method** - **Use relative imports** - **Use the `dataclass` decorator from `dataclasses`** - **Group config changes in the release notes** - **Sort `__all__`** - **Remove unnecessary required from metadata**
2 parents 54735f7 + c7d4da4 commit b0a656e

File tree

4 files changed

+41
-36
lines changed

4 files changed

+41
-36
lines changed

RELEASE_NOTES.md

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,25 @@
66

77
## Upgrading
88

9-
- The `LoggingConfigUpdater` was renamed to `LoggingConfigUpdatingActor` to follow the actor naming convention.
9+
- `frequenz.sdk.config`
10+
11+
* `LoggingConfigUpdater`
12+
13+
+ Renamed to `LoggingConfigUpdatingActor` to follow the actor naming convention.
14+
+ Make all arguments to the constructor keyword-only.
15+
16+
* `LoggingConfig`
17+
18+
+ The `load()` method was removed. Please use `frequenz.sdk.config.load_config()` instead.
19+
+ The class is now a standard `dataclass` instead of a `marshmallow_dataclass`.
20+
21+
* `LoggerConfig` is not a standard `dataclass` instead of a `marshmallow_dataclass`.
1022

1123
## New Features
1224

13-
<!-- Here goes the main new features and examples or instructions on how to use them -->
25+
- `LoggingConfigUpdatingActor`
26+
27+
* Added a new `name` argument to the constructor to be able to override the actor's name.
1428

1529
## Bug Fixes
1630

src/frequenz/sdk/config/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99

1010
__all__ = [
1111
"ConfigManagingActor",
12-
"LoggingConfig",
1312
"LoggerConfig",
13+
"LoggingConfig",
1414
"LoggingConfigUpdatingActor",
1515
"load_config",
1616
]

src/frequenz/sdk/config/_logging_actor.py

Lines changed: 14 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,15 @@
55

66
import logging
77
from collections.abc import Mapping
8-
from dataclasses import field
9-
from typing import Annotated, Any, Self, cast
8+
from dataclasses import dataclass, field
9+
from typing import Annotated, Any
1010

1111
import marshmallow
1212
import marshmallow.validate
1313
from frequenz.channels import Receiver
14-
from marshmallow import RAISE
15-
from marshmallow_dataclass import class_schema, dataclass
1614

17-
from frequenz.sdk.actor import Actor
15+
from ..actor import Actor
16+
from ._util import load_config
1817

1918
_logger = logging.getLogger(__name__)
2019

@@ -24,6 +23,7 @@
2423
validate=marshmallow.validate.OneOf(choices=logging.getLevelNamesMapping())
2524
),
2625
]
26+
"""A marshmallow field for validating log levels."""
2727

2828

2929
@dataclass
@@ -36,7 +36,6 @@ class LoggerConfig:
3636
"metadata": {
3737
"description": "Log level for the logger. Uses standard logging levels."
3838
},
39-
"required": False,
4039
},
4140
)
4241
"""The log level for the logger."""
@@ -52,7 +51,6 @@ class LoggingConfig:
5251
"metadata": {
5352
"description": "Default default configuration for all loggers.",
5453
},
55-
"required": False,
5654
},
5755
)
5856
"""The default log level."""
@@ -63,27 +61,10 @@ class LoggingConfig:
6361
"metadata": {
6462
"description": "Configuration for a logger (the key is the logger name)."
6563
},
66-
"required": False,
6764
},
6865
)
6966
"""The list of loggers configurations."""
7067

71-
@classmethod
72-
def load(cls, configs: Mapping[str, Any]) -> Self: # noqa: DOC502
73-
"""Load and validate configs from a dictionary.
74-
75-
Args:
76-
configs: The configuration to validate.
77-
78-
Returns:
79-
The configuration if they are valid.
80-
81-
Raises:
82-
ValidationError: if the configuration are invalid.
83-
"""
84-
schema = class_schema(cls)()
85-
return cast(Self, schema.load(configs, unknown=RAISE))
86-
8768

8869
class LoggingConfigUpdatingActor(Actor):
8970
"""Actor that listens for logging configuration changes and sets them.
@@ -133,31 +114,36 @@ async def run() -> None:
133114

134115
def __init__(
135116
self,
117+
*,
136118
config_recv: Receiver[Mapping[str, Any]],
137-
log_format: str = "%(asctime)s %(levelname)-8s %(name)s:%(lineno)s: %(message)s",
138119
log_datefmt: str = "%Y-%m-%dT%H:%M:%S%z",
120+
log_format: str = "%(asctime)s %(levelname)-8s %(name)s:%(lineno)s: %(message)s",
121+
name: str | None = None,
139122
):
140123
"""Initialize this instance.
141124
142125
Args:
143126
config_recv: The receiver to listen for configuration changes.
144-
log_format: Use the specified format string in logs.
145127
log_datefmt: Use the specified date/time format in logs.
128+
log_format: Use the specified format string in logs.
129+
name: The name of this actor. If `None`, `str(id(self))` will be used. This
130+
is used mostly for debugging purposes.
146131
147132
Note:
148133
The `log_format` and `log_datefmt` parameters are used in a call to
149134
`logging.basicConfig()`. If logging has already been configured elsewhere
150135
in the application (through a previous `basicConfig()` call), then the format
151136
settings specified here will be ignored.
152137
"""
153-
super().__init__()
154138
self._config_recv = config_recv
155139

156140
# Setup default configuration.
157141
# This ensures logging is configured even if actor fails to start or
158142
# if the configuration cannot be loaded.
159143
self._current_config: LoggingConfig = LoggingConfig()
160144

145+
super().__init__(name=name)
146+
161147
logging.basicConfig(
162148
format=log_format,
163149
datefmt=log_datefmt,
@@ -168,7 +154,7 @@ async def _run(self) -> None:
168154
"""Listen for configuration changes and update logging."""
169155
async for message in self._config_recv:
170156
try:
171-
new_config = LoggingConfig.load(message)
157+
new_config = load_config(LoggingConfig, message)
172158
except marshmallow.ValidationError:
173159
_logger.exception(
174160
"Invalid logging configuration received. Skipping config update"

tests/config/test_logging_actor.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,12 @@
1313
from marshmallow import ValidationError
1414
from pytest_mock import MockerFixture
1515

16-
from frequenz.sdk.config import LoggerConfig, LoggingConfig, LoggingConfigUpdatingActor
16+
from frequenz.sdk.config import (
17+
LoggerConfig,
18+
LoggingConfig,
19+
LoggingConfigUpdatingActor,
20+
load_config,
21+
)
1722

1823

1924
def test_logging_config() -> None:
@@ -33,19 +38,19 @@ def test_logging_config() -> None:
3338
},
3439
)
3540

36-
assert LoggingConfig.load(config_raw) == config
41+
assert load_config(LoggingConfig, config_raw) == config
3742

3843
config_raw = {}
3944
config = LoggingConfig()
40-
assert LoggingConfig.load(config_raw) == config
45+
assert load_config(LoggingConfig, config_raw) == config
4146

4247
config_raw = {"root_logger": {"level": "UNKNOWN"}}
4348
with pytest.raises(ValidationError):
44-
LoggingConfig.load(config_raw)
49+
load_config(LoggingConfig, config_raw)
4550

4651
config_raw = {"unknown": {"frequenz.sdk.actor": {"level": "DEBUG"}}}
4752
with pytest.raises(ValidationError):
48-
LoggingConfig.load(config_raw)
53+
load_config(LoggingConfig, config_raw)
4954

5055

5156
@pytest.fixture

0 commit comments

Comments
 (0)