Skip to content

Commit 900fa0e

Browse files
authored
Raise ValidationError from the config test validators (#1456)
CI started failing on `v1.x.x` yesterday with three unrelated-looking config test failures. They come from the `frequenz-quantities` 1.0.2 release, which raised its `marshmallow` floor to 4 (frequenz-floss/frequenz-quantities-python#107) and thereby dragged `marshmallow` 4 into our environment for the first time. Marshmallow 4 dropped support for validators that return `False` instead of raising, which is what the config test dataclasses were doing, so the validation silently stopped happening. The fix is test-only, but there is a release notes entry because the same silent acceptance hits anyone who wrote boolean validators for their own config dataclasses. Heads-up for whoever reviews: the docs job on this PR will stay red, for a second and unrelated problem from the same `frequenz-quantities` 1.0.2 release. That one is a docstring in `frequenz-quantities` using a relative mkdocstrings cross-reference, which misresolves whenever it is rendered under a path other than the one it was written in, as happens here with `BaseConfigSchema` inheriting `QuantitySchema.TYPE_MAPPING`. It cannot be fixed on this side, only worked around by dropping documentation, and frequenz-floss/frequenz-quantities-python#170 already fixes it upstream. #1455 is blocked on the same thing.
2 parents 9e92877 + 280f78d commit 900fa0e

3 files changed

Lines changed: 31 additions & 3 deletions

File tree

‎RELEASE_NOTES.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66

77
## Upgrading
88

9+
- Custom validators passed through a configuration dataclass field's `metadata` (as used by `load_config()` and `ConfigManager.new_receiver()`) must now raise a `marshmallow.ValidationError` to reject a value. `marshmallow` 4 removed support for validators that signal failure by returning `False`, so such validators are now ignored and the invalid value is accepted silently. This affects you as soon as `marshmallow` 4 is installed, which `frequenz-quantities` 1.0.2 and later require.
10+
911
<!-- Here goes notes on how to upgrade from previous versions, including deprecations and what they should be replaced with -->
1012

1113
## New Features

‎tests/config/test_manager.py‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,24 @@
2020
from frequenz.sdk.config._manager import _get_key
2121

2222

23+
def _validate_name(value: str) -> None:
24+
"""Validate that a name starts with `test`.
25+
26+
Args:
27+
value: The name to validate.
28+
29+
Raises:
30+
marshmallow.ValidationError: If the name doesn't start with `test`.
31+
"""
32+
if not value.startswith("test"):
33+
raise marshmallow.ValidationError("Invalid value.")
34+
35+
2336
@dataclass
2437
class SimpleConfig:
2538
"""A simple configuration class for testing."""
2639

27-
name: str = dataclasses.field(metadata={"validate": lambda s: s.startswith("test")})
40+
name: str = dataclasses.field(metadata={"validate": _validate_name})
2841
value: int
2942

3043

‎tests/config/test_util.py‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,19 +14,32 @@
1414
from frequenz.sdk.config._util import load_config
1515

1616

17+
def _validate_name(value: str) -> None:
18+
"""Validate that a name starts with `test`.
19+
20+
Args:
21+
value: The name to validate.
22+
23+
Raises:
24+
marshmallow.ValidationError: If the name doesn't start with `test`.
25+
"""
26+
if not value.startswith("test"):
27+
raise marshmallow.ValidationError("Invalid value.")
28+
29+
1730
@dataclasses.dataclass
1831
class SimpleConfig:
1932
"""A simple configuration class for testing."""
2033

21-
name: str = dataclasses.field(metadata={"validate": lambda s: s.startswith("test")})
34+
name: str = dataclasses.field(metadata={"validate": _validate_name})
2235
value: int
2336

2437

2538
@marshmallow_dataclass.dataclass
2639
class MmSimpleConfig:
2740
"""A simple marshmallow_dataclass configuration class for testing."""
2841

29-
name: str = dataclasses.field(metadata={"validate": lambda s: s.startswith("test")})
42+
name: str = dataclasses.field(metadata={"validate": _validate_name})
3043
value: int
3144

3245

0 commit comments

Comments
 (0)