Skip to content

Commit c117c0e

Browse files
committed
Make arguments passed to marshmallow explicit
Forwarding keyword arguments looks nice but makes the function typing much weaker. Any typos in the keyword arguments taken by the function end up undetected until they reach `marshmallow.Schema.load()` and then they give an obscure error about some unexpected argument that was never the intention to forward to `marshmallow`. Signed-off-by: Leandro Lucarella <[email protected]>
1 parent f0b457c commit c117c0e

File tree

3 files changed

+12
-5
lines changed

3 files changed

+12
-5
lines changed

RELEASE_NOTES.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@
2626
+ The class is now immutable.
2727
+ The constructor now accepts only keyword arguments.
2828

29-
* `load_config()`: the `base_schema` argument is now keyword-only.
29+
* `load_config()`:
30+
31+
+ The `base_schema` argument is now keyword-only.
32+
+ The arguments forwarded to `marshmallow.Schema.load()` now must be passed explicitly via the `marshmallow_load_kwargs` argument, as a `dict` to improve the type-checking.
3033

3134
## New Features
3235

src/frequenz/sdk/config/_util.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ def load_config(
3434
/,
3535
*,
3636
base_schema: type[Schema] | None = None,
37-
**marshmallow_load_kwargs: Any,
37+
marshmallow_load_kwargs: dict[str, Any] | None = None,
3838
) -> DataclassT:
3939
"""Load a configuration from a dictionary into an instance of a configuration class.
4040
@@ -63,13 +63,15 @@ def load_config(
6363
base_schema: An optional class to be used as a base schema for the configuration
6464
class. This allow using custom fields for example. Will be passed to
6565
[`marshmallow_dataclass.class_schema`][].
66-
**marshmallow_load_kwargs: Additional arguments to be passed to
66+
marshmallow_load_kwargs: Additional arguments to be passed to
6767
[`marshmallow.Schema.load`][].
6868
6969
Returns:
7070
The loaded configuration as an instance of the configuration class.
7171
"""
72-
instance = class_schema(cls, base_schema)().load(config, **marshmallow_load_kwargs)
72+
instance = class_schema(cls, base_schema)().load(
73+
config, **(marshmallow_load_kwargs or {})
74+
)
7375
# We need to cast because `.load()` comes from marshmallow and doesn't know which
7476
# type is returned.
7577
return cast(DataclassT, instance)

tests/config/test_util.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,9 @@ def test_load_config_type_hints(mocker: MockerFixture) -> None:
100100
config: dict[str, Any] = {}
101101

102102
# We add the type hint to test that the return type (hint) is correct
103-
_: SimpleConfig = load_config(SimpleConfig, config, marshmallow_arg=1)
103+
_: SimpleConfig = load_config(
104+
SimpleConfig, config, marshmallow_load_kwargs={"marshmallow_arg": 1}
105+
)
104106
mock_class_schema.return_value.load.assert_called_once_with(
105107
config, marshmallow_arg=1
106108
)

0 commit comments

Comments
 (0)