From f882f62164534b34c14aa4a858bc5a58cd296761 Mon Sep 17 00:00:00 2001 From: r0b2g1t Date: Sun, 13 Sep 2026 13:42:21 +0200 Subject: [PATCH] fix: runtime config update ignored by config file Runtime settings were handed back to pydantic-settings as init settings, which rank below the config file and the environment. Any key already present in EOS.config.json or in the environment silently discarded the update, so a bulk PUT /v1/config returned 200 without applying anything, while the granular PUT /v1/config/{path} endpoint kept working. Add a dedicated runtime settings source ranked directly below the command line arguments and record granular updates there as well, so both endpoints share one store that survives re-evaluation of the settings sources. Environment variables keep precedence over the config file for all keys that were not set at runtime. Also repairs revert_settings() and update(), which passed their data through the same init settings. Closes #1303 --- docs/akkudoktoreos/configuration.md | 13 ++- src/akkudoktoreos/config/config.py | 81 ++++++++++++++--- src/akkudoktoreos/core/pydantic.py | 48 ++++++---- tests/test_config.py | 131 ++++++++++++++++++++++++++++ 4 files changed, 239 insertions(+), 34 deletions(-) diff --git a/docs/akkudoktoreos/configuration.md b/docs/akkudoktoreos/configuration.md index 65933c54..9fc2cd3a 100644 --- a/docs/akkudoktoreos/configuration.md +++ b/docs/akkudoktoreos/configuration.md @@ -40,10 +40,15 @@ Use endpoint `POST /v1/config/reset` to reset the configuration to the values in The configuration sources and their priorities are as follows: -1. `Settings`: Provided during runtime by the REST interface -2. `Environment Variables`: Defined at startup of the REST server and during runtime -3. `EOS Configuration File`: Read at startup of the REST server and on request -4. `Default Values` +1. `Command Line Arguments`: Provided at startup of the REST server +2. `Settings`: Provided during runtime by the REST interface +3. `Environment Variables`: Defined at startup of the REST server and during runtime +4. `EOS Configuration File`: Read at startup of the REST server and on request +5. `Default Values` + +Runtime settings are kept until they are reset by `POST /v1/config/reset`. All other sources are +re-evaluated on every configuration change, which keeps environment variable changes effective for +all configuration keys that were not set during runtime. ### Runtime Config Updates diff --git a/src/akkudoktoreos/config/config.py b/src/akkudoktoreos/config/config.py index ad49f366..932a2c2d 100644 --- a/src/akkudoktoreos/config/config.py +++ b/src/akkudoktoreos/config/config.py @@ -13,6 +13,7 @@ import json import os import sys import tempfile +from copy import deepcopy from pathlib import Path from typing import Any, Callable, ClassVar, Optional, Type, Union @@ -38,7 +39,7 @@ from akkudoktoreos.core.emsettings import ( ) from akkudoktoreos.core.logabc import LOGGING_LEVELS from akkudoktoreos.core.logsettings import LoggingCommonSettings -from akkudoktoreos.core.pydantic import PydanticModelNestedValueMixin, merge_models +from akkudoktoreos.core.pydantic import PydanticModelNestedValueMixin, deep_merge from akkudoktoreos.core.version import __version__ from akkudoktoreos.devices.devices import DevicesCommonSettings from akkudoktoreos.measurement.measurement import MeasurementCommonSettings @@ -396,6 +397,8 @@ class ConfigEOS(SingletonMixin, SettingsEOSDefaults): } _config_file_path: ClassVar[Optional[Path]] = None _config_autosave: ClassVar[str] = "" + # Settings provided at runtime, e.g. by the REST interface. Highest priority after CLI. + _runtime_settings: ClassVar[dict[str, Any]] = {} _force_documentation_mode = False def __hash__(self) -> int: @@ -519,6 +522,14 @@ class ConfigEOS(SingletonMixin, SettingsEOSDefaults): return settings + def lazy_runtime_settings() -> dict: + """Runtime settings. + + Settings provided during runtime, e.g. by the REST interface. They supersede any + setting from the environment or the configuration file until they are reset. + """ + return deepcopy(cls._runtime_settings) + def lazy_config_file_settings() -> dict: """Config file settings. @@ -668,6 +679,7 @@ class ConfigEOS(SingletonMixin, SettingsEOSDefaults): # runtime configuration. setting_sources = [ lazy_config_cli_settings, # Prio high + lazy_runtime_settings, # settings provided during runtime lazy_env_settings, lazy_dotenv_settings, lazy_config_file_settings, # resolves/creates config file path @@ -741,11 +753,10 @@ class ConfigEOS(SingletonMixin, SettingsEOSDefaults): ) def merge_settings_from_dict(self, data: dict) -> None: - """Merges the provided dictionary data into the current instance. + """Merges the provided dictionary data into the runtime settings. - Creates a new settings instance, then applies the dictionary data through validation, - and finally merges the validated settings into the current instance. None values - are not merged. + The data is added to the runtime settings, which have priority over the environment and + the EOS configuration file. All configuration sources are re-evaluated afterwards. Args: data (dict): Dictionary containing field values to merge into the @@ -762,18 +773,58 @@ class ConfigEOS(SingletonMixin, SettingsEOSDefaults): config.merge_settings_from_dict(new_data) """ - merged = merge_models( - self, - data, - ) + previous_settings = ConfigEOS._runtime_settings + ConfigEOS._runtime_settings = deep_merge(previous_settings, deepcopy(data)) + try: + self._setup() + except Exception: + # Keep the runtime settings in sync with the actual configuration + ConfigEOS._runtime_settings = previous_settings + raise - self._setup(**merged) + def set_nested_value(self, path: str, value: Any) -> None: + """Set a nested configuration value and remember it as runtime setting. + + Args: + path (str): A '/'-separated path to the nested attribute (e.g. "server/port"). + value (Any): The new value to set. + """ + super().set_nested_value(path, value) + + # Remember as runtime setting to survive re-evaluation of the configuration sources. + # List indices can not be expressed by the settings dictionary - remember the whole list. + keys = [] + for key in path.strip("/").split("/"): + if key.isdigit(): + break + keys.append(key) + setting = self.get_nested_value("/".join(keys)) + if isinstance(setting, SettingsBaseModel): + setting = setting.model_dump( + exclude_none=True, exclude_unset=True, exclude_computed_fields=True + ) + elif isinstance(setting, list): + setting = [ + item.model_dump( + exclude_none=True, exclude_unset=True, exclude_computed_fields=True + ) + if isinstance(item, SettingsBaseModel) + else item + for item in setting + ] + runtime_setting: dict[str, Any] = {} + node = runtime_setting + for key in keys[:-1]: + node = node.setdefault(key, {}) + node[keys[-1]] = setting + ConfigEOS._runtime_settings = deep_merge(ConfigEOS._runtime_settings, runtime_setting) def reset_settings(self) -> None: """Reset all changed settings to environment/config file defaults. This functions basically deletes the settings provided before. """ + ConfigEOS._runtime_settings = {} self._setup() def revert_settings(self, backup_id: str) -> None: @@ -812,7 +863,11 @@ class ConfigEOS(SingletonMixin, SettingsEOSDefaults): backup_data: dict[str, Any] = json.load(f) backup_settings = migrate_config_data(backup_data) - self._setup(**backup_settings.model_dump(exclude_none=True, exclude_unset=True)) + # Backup settings are runtime settings - they supersede environment and config file. + ConfigEOS._runtime_settings = backup_settings.model_dump( + exclude_none=True, exclude_defaults=True, exclude_computed_fields=True + ) + self._setup() def list_backups(self) -> dict[str, dict[str, Any]]: """List available configuration backup files and extract metadata. @@ -1094,11 +1149,11 @@ class ConfigEOS(SingletonMixin, SettingsEOSDefaults): """Updates all configuration fields. This method updates all configuration fields using the following order for value retrieval: - 1. Current settings. + 1. Runtime settings. 2. Environment variables. 3. EOS configuration file. 4. Field default constants. The first non None value in priority order is taken. """ - self._setup(**self.model_dump()) + self._setup() diff --git a/src/akkudoktoreos/core/pydantic.py b/src/akkudoktoreos/core/pydantic.py index 924d042c..907cdcf7 100644 --- a/src/akkudoktoreos/core/pydantic.py +++ b/src/akkudoktoreos/core/pydantic.py @@ -65,6 +65,37 @@ from akkudoktoreos.utils.datetimeutil import ( _model_private_state: "weakref.WeakKeyDictionary[Union[PydanticBaseModel, PydanticModelNestedValueMixin], Dict[str, Any]]" = weakref.WeakKeyDictionary() +def deep_merge(source_data: Any, update_data: Any) -> Any: + """Merge two data structures recursively. + + Values in update_data (including None) override source values. + Nested dictionaries are merged recursively. + Lists in update_data replace source lists entirely. + + Args: + source_data (Any): Data to merge into. + update_data (Any): Data to merge from. + + Returns: + Any: The merged data. + """ + if isinstance(source_data, dict) and isinstance(update_data, dict): + merged = dict(source_data) + for key, update_value in update_data.items(): + if key in merged: + merged[key] = deep_merge(merged[key], update_value) + else: + merged[key] = update_value + return merged + + # If both are lists, replace source list with update list + if isinstance(source_data, list) and isinstance(update_data, list): + return update_data + + # For other types or if update_data is None, override source_data + return update_data + + def merge_models(source: BaseModel, update_dict: dict[str, Any]) -> dict[str, Any]: """Merge a Pydantic model instance with an update dictionary. @@ -83,23 +114,6 @@ def merge_models(source: BaseModel, update_dict: dict[str, Any]) -> dict[str, An dict[str, Any]: Merged dictionary representing combined model data. """ - def deep_merge(source_data: Any, update_data: Any) -> Any: - if isinstance(source_data, dict) and isinstance(update_data, dict): - merged = dict(source_data) - for key, update_value in update_data.items(): - if key in merged: - merged[key] = deep_merge(merged[key], update_value) - else: - merged[key] = update_value - return merged - - # If both are lists, replace source list with update list - if isinstance(source_data, list) and isinstance(update_data, list): - return update_data - - # For other types or if update_data is None, override source_data - return update_data - source_dict = source.model_dump( exclude_unset=True, exclude_computed_fields=True, diff --git a/tests/test_config.py b/tests/test_config.py index 205b7172..31bbd86f 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,3 +1,4 @@ +import json import tempfile from pathlib import Path from typing import Any, Optional, Union @@ -547,3 +548,133 @@ def test_merge_settings_empty(config_eos): config_eos.merge_settings_from_dict({}) # No changes assert config_eos.general.latitude == original_latitude # Should remain unchanged + + +# ------------------------------------ +# Runtime settings priority (issue #1303) +# ------------------------------------ + + +@pytest.fixture +def config_eos_file(config_eos_factory) -> ConfigEOS: + """ConfigEOS with the EOS configuration file as an active settings source.""" + return config_eos_factory( + init={ + "with_init_settings": True, + "with_env_settings": True, + "with_dotenv_settings": False, + "with_file_settings": True, + "with_file_secret_settings": False, + } + ) + + +def write_config_file(config_eos: ConfigEOS, settings: dict[str, Any]) -> None: + """Write settings to the EOS configuration file and load them.""" + settings = {"general": {"version": config_eos.general.version}, **settings} + config_file_path = config_eos.general.config_file_path + assert config_file_path is not None + config_file_path.write_text(json.dumps(settings), encoding="utf-8") + config_eos.reset_settings() + + +def test_merge_settings_overrides_config_file(config_eos_file): + """Runtime settings take precedence over the EOS configuration file.""" + write_config_file( + config_eos_file, + { + "optimization": {"genetic": {"individuals": 200}}, + "pvforecast": { + "planes": [ + {"surface_tilt": 30.0, "surface_azimuth": azimuth, "peakpower": 5.0} + for azimuth in (0.0, 90.0, 180.0, 270.0) + ] + }, + }, + ) + assert config_eos_file.optimization.genetic.individuals == 200 + assert len(config_eos_file.pvforecast.planes) == 4 + + config_eos_file.merge_settings_from_dict( + { + "optimization": {"genetic": {"individuals": 300}}, + "pvforecast": { + "planes": [{"surface_tilt": 30.0, "surface_azimuth": 180.0, "peakpower": 5.0}] + }, + } + ) + + assert config_eos_file.optimization.genetic.individuals == 300 + assert len(config_eos_file.pvforecast.planes) == 1 + + +def test_merge_settings_overrides_env(config_eos_file, monkeypatch): + """Runtime settings take precedence over environment variables.""" + monkeypatch.setenv("EOS_OPTIMIZATION__GENETIC__INDIVIDUALS", "150") + config_eos_file.reset_settings() + assert config_eos_file.optimization.genetic.individuals == 150 + + config_eos_file.merge_settings_from_dict({"optimization": {"genetic": {"individuals": 300}}}) + + assert config_eos_file.optimization.genetic.individuals == 300 + + +def test_env_overrides_config_file_after_merge(config_eos_file, monkeypatch): + """Environment variables keep precedence over the config file for untouched keys.""" + write_config_file(config_eos_file, {"server": {"port": 9000}}) + monkeypatch.setenv("EOS_SERVER__PORT", "9500") + config_eos_file.reset_settings() + assert config_eos_file.server.port == 9500 + + # A runtime update of an unrelated key must not freeze the env value + config_eos_file.merge_settings_from_dict({"general": {"latitude": 51.1657}}) + assert config_eos_file.general.latitude == 51.1657 + assert config_eos_file.server.port == 9500 + + monkeypatch.setenv("EOS_SERVER__PORT", "9600") + config_eos_file.reset_settings() + assert config_eos_file.server.port == 9600 + + +def test_reset_settings_drops_runtime_settings(config_eos_file): + """Reset drops runtime settings and falls back to the config file.""" + write_config_file(config_eos_file, {"optimization": {"genetic": {"individuals": 200}}}) + + config_eos_file.merge_settings_from_dict({"optimization": {"genetic": {"individuals": 300}}}) + assert config_eos_file.optimization.genetic.individuals == 300 + + config_eos_file.reset_settings() + assert config_eos_file.optimization.genetic.individuals == 200 + + +def test_set_nested_value_survives_merge(config_eos_file): + """Granular updates are not lost by a later bulk update.""" + write_config_file(config_eos_file, {"optimization": {"genetic": {"individuals": 200}}}) + + config_eos_file.set_nested_value("optimization/genetic/individuals", 400) + assert config_eos_file.optimization.genetic.individuals == 400 + + config_eos_file.merge_settings_from_dict({"general": {"latitude": 51.1657}}) + assert config_eos_file.optimization.genetic.individuals == 400 + + +def test_revert_settings_restores_backup(config_eos_file): + """Revert restores the backup values even if the config file differs.""" + write_config_file(config_eos_file, {"optimization": {"genetic": {"individuals": 200}}}) + + config_file_path = config_eos_file.general.config_file_path + assert config_file_path is not None + backup_path = config_file_path.with_suffix(".backup") + backup_path.write_text( + json.dumps( + { + "general": {"version": config_eos_file.general.version}, + "optimization": {"genetic": {"individuals": 500}}, + } + ), + encoding="utf-8", + ) + + config_eos_file.revert_settings("backup") + + assert config_eos_file.optimization.genetic.individuals == 500