Skip to content

Raise ValidationError from the config test validators - #1456

Merged
llucax merged 1 commit into
frequenz-floss:v1.x.xfrom
llucax:fix-marshmallow-4-validators
Sep 8, 2026
Merged

Raise ValidationError from the config test validators#1456
llucax merged 1 commit into
frequenz-floss:v1.x.xfrom
llucax:fix-marshmallow-4-validators

Conversation

@llucax

@llucax llucax commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

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.

`frequenz-quantities` 1.0.2 changed its `marshmallow` optional dependency
floor from `< 4` to `>= 4` (frequenz-floss/frequenz-quantities-python#107),
correcting metadata that had been wrong since that library was ported to
`marshmallow` 4.  That cap was the only thing holding us on `marshmallow`
3, as our own requirement has allowed `< 5` since 870e880, so CI now
resolves `marshmallow` 4.

`marshmallow` 4 removed support for validators that signal failure by
returning `False`, deprecated in 3.24.0:

    Custom validators must raise a `ValidationError` for invalid values
    (deprecated in 3.24.0).  Returning `False` is no longer supported.

The configuration test dataclasses used exactly that pattern, a
`lambda s: s.startswith("test")` in the field metadata.  The validator is
still registered and still called, only its return value is ignored, so
the invalid value is accepted and `test_load_config_dataclass` and
`test_new_receiver_configurations[Validation Error]` fail.

Replacing the lambdas with a function that raises restores the checks.
The message stays `Invalid value.`, which is what `marshmallow` 3 emitted
through its `validator_failed` error key, so the expected output in
`test_manager.py` is unchanged and the surrounding test cases remain
consistent in asserting on `marshmallow`'s stock messages.

Nothing under `src/` needed changing: the validators there are
`marshmallow.validate.Range` and `OneOf`, which already raise.  A raising
validator also works on `marshmallow` 3, so the supported range is not
narrowed, but the same silent acceptance can hit users who wrote their
own boolean validators, hence the release notes entry.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@llucax
llucax requested a review from a team as a code owner September 8, 2026 10:21
@llucax
llucax removed the request for review from a team September 8, 2026 10:21
@llucax
llucax requested a review from shsms September 8, 2026 10:21
@github-actions github-actions Bot added part:docs Affects the documentation part:tests Affects the unit, integration and performance (benchmarks) tests labels Sep 8, 2026
@llucax llucax added the scope:breaking-change Breaking change, users will need to update their code label Sep 8, 2026
@llucax
llucax requested a review from Marenz September 8, 2026 10:28
@llucax
llucax enabled auto-merge September 8, 2026 11:49
@llucax llucax self-assigned this Sep 8, 2026
@llucax

llucax commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

quantities 1.0.3 is out, this is now green.

I think we should release this as a "major release", as this breaking change will drag anyone using config schemas too.

@llucax llucax added this to the v1.0.0-rc2300 milestone Sep 8, 2026
@github-project-automation github-project-automation Bot moved this from To do to Review approved in Python SDK Roadmap Sep 8, 2026
@llucax
llucax added this pull request to the merge queue Sep 8, 2026
Merged via the queue into frequenz-floss:v1.x.x with commit 900fa0e Sep 8, 2026
15 of 17 checks passed
@llucax
llucax deleted the fix-marshmallow-4-validators branch September 8, 2026 12:36
@github-project-automation github-project-automation Bot moved this from Review approved to Done in Python SDK Roadmap Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

part:docs Affects the documentation part:tests Affects the unit, integration and performance (benchmarks) tests scope:breaking-change Breaking change, users will need to update their code

Projects

Development

Successfully merging this pull request may close these issues.

2 participants