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

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
@llucax llucax modified the milestones: v1.0.0-rc2300, v1.0.0-rc2212 Sep 15, 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