Skip to content

WIP: Revamp config management #1104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 25 commits into from
Closed

WIP: Revamp config management #1104

Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
2d282e4
Bump the minimum required channels version to v1.4.0
llucax Nov 22, 2024
b67acd6
Add a `ConfigManager` class
llucax Nov 15, 2024
29725c4
Add an option to wait for the first configuration
llucax Nov 22, 2024
2921ed8
Add option to only send changed configurations
llucax Nov 22, 2024
751d20e
Add support for subscribing to specific config keys
llucax Nov 22, 2024
3ccb4eb
Add support for validating configurations with a schema
llucax Nov 28, 2024
e39cd6b
Add support to filter a sub-key
llucax Nov 25, 2024
3129c42
Assert the receiver has the correct type
llucax Dec 6, 2024
63c96a0
Add support for skipping `None` configs
llucax Dec 6, 2024
77295dc
Add a global instance for the config manager
llucax Nov 22, 2024
cd08feb
Support using `BackgroundService` as a *mixin*
llucax Dec 6, 2024
fbe18de
Add a base config schema that provides quantities support
llucax Dec 6, 2024
3201348
Add a `Reconfigurable` *mixin*
llucax Dec 6, 2024
2878a2a
Make the `LoggingConfigUpdatingActor` `Reconfigurable`
llucax Dec 9, 2024
0b54413
Allow configuring logging via `ConfigManager`
llucax Dec 9, 2024
80fc626
Revert "Make the `LoggingConfigUpdatingActor` `Reconfigurable`"
llucax Dec 10, 2024
c34c199
Revert "Add a `Reconfigurable` *mixin*"
llucax Dec 10, 2024
05164ef
Revert "Support using `BackgroundService` as a *mixin*"
llucax Dec 10, 2024
dcd76fb
Revert "Add a global instance for the config manager"
llucax Dec 10, 2024
9215c15
WIP: Add full example in the `config` module.
llucax Nov 22, 2024
40dcc3f
Revert "Add an option to wait for the first configuration"
llucax Dec 10, 2024
1f96cec
Improve logging for configuration file reading
llucax Dec 10, 2024
61d4b11
Remove support for receiving raw mapping as configuration
llucax Dec 10, 2024
0cc7e94
Move note about update bursts to Skipping superfluous updates
llucax Dec 10, 2024
4189066
Exclude unknown fields from the config by default
llucax Dec 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/frequenz/sdk/config/_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,10 @@ def new_receiver( # noqa: DOC502
Additional arguments can be passed to [`marshmallow.Schema.load`][] using
the `marshmallow_load_kwargs` keyword arguments.

If unspecified, the `marshmallow_load_kwargs` will have the `unknown` key set to
[`marshmallow.EXCLUDE`][] (instead of the normal [`marshmallow.RAISE`][]
default).

### Skipping superfluous updates

If there is a burst of configuration updates, the receiver will only receive the
Expand Down Expand Up @@ -317,6 +321,9 @@ def _is_dataclass(config: DataclassT | None) -> TypeGuard[DataclassT]:
"""Return whether the configuration is a dataclass."""
return config is not None

if "unknown" not in marshmallow_load_kwargs:
marshmallow_load_kwargs["unknown"] = marshmallow.EXCLUDE

Comment on lines +324 to +326
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we would like to raise exception if there are any unknown fields.

RAISE (default): raise a ValidationError if there are any unknown fields
EXCLUDE: exclude unknown fields
INCLUDE: accept and include the unknown fields

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's too restrictive for a config. For example, if you deploy a new version that has new options, and you configure one of the new options. Then you find a regression and want to roll back. If you don't rollback the config too, then your app doesn't start anymore.

In the new version I'm actually logging warnings if unknown fields are found, I think that's the most balanced approach, as having unknown fields go unnoticed could also cause issues (a typo in a config variable name might end up having a misconfigured app without noticing).

recv_name = f"{self}_receiver" if key is None else f"{self}_receiver_{key}"
receiver = self.config_channel.new_receiver(name=recv_name, limit=1)

Expand Down
Loading