mirror of
https://github.com/Akkudoktor-EOS/EOS.git
synced 2026-10-16 02:51:56 +00:00
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
This commit is contained in:
@@ -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:
|
The configuration sources and their priorities are as follows:
|
||||||
|
|
||||||
1. `Settings`: Provided during runtime by the REST interface
|
1. `Command Line Arguments`: Provided at startup of the REST server
|
||||||
2. `Environment Variables`: Defined at startup of the REST server and during runtime
|
2. `Settings`: Provided during runtime by the REST interface
|
||||||
3. `EOS Configuration File`: Read at startup of the REST server and on request
|
3. `Environment Variables`: Defined at startup of the REST server and during runtime
|
||||||
4. `Default Values`
|
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
|
### Runtime Config Updates
|
||||||
|
|
||||||
|
|||||||
@@ -13,6 +13,7 @@ import json
|
|||||||
import os
|
import os
|
||||||
import sys
|
import sys
|
||||||
import tempfile
|
import tempfile
|
||||||
|
from copy import deepcopy
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import Any, Callable, ClassVar, Optional, Type, Union
|
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.logabc import LOGGING_LEVELS
|
||||||
from akkudoktoreos.core.logsettings import LoggingCommonSettings
|
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.core.version import __version__
|
||||||
from akkudoktoreos.devices.devices import DevicesCommonSettings
|
from akkudoktoreos.devices.devices import DevicesCommonSettings
|
||||||
from akkudoktoreos.measurement.measurement import MeasurementCommonSettings
|
from akkudoktoreos.measurement.measurement import MeasurementCommonSettings
|
||||||
@@ -396,6 +397,8 @@ class ConfigEOS(SingletonMixin, SettingsEOSDefaults):
|
|||||||
}
|
}
|
||||||
_config_file_path: ClassVar[Optional[Path]] = None
|
_config_file_path: ClassVar[Optional[Path]] = None
|
||||||
_config_autosave: ClassVar[str] = ""
|
_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
|
_force_documentation_mode = False
|
||||||
|
|
||||||
def __hash__(self) -> int:
|
def __hash__(self) -> int:
|
||||||
@@ -519,6 +522,14 @@ class ConfigEOS(SingletonMixin, SettingsEOSDefaults):
|
|||||||
|
|
||||||
return settings
|
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:
|
def lazy_config_file_settings() -> dict:
|
||||||
"""Config file settings.
|
"""Config file settings.
|
||||||
|
|
||||||
@@ -668,6 +679,7 @@ class ConfigEOS(SingletonMixin, SettingsEOSDefaults):
|
|||||||
# runtime configuration.
|
# runtime configuration.
|
||||||
setting_sources = [
|
setting_sources = [
|
||||||
lazy_config_cli_settings, # Prio high
|
lazy_config_cli_settings, # Prio high
|
||||||
|
lazy_runtime_settings, # settings provided during runtime
|
||||||
lazy_env_settings,
|
lazy_env_settings,
|
||||||
lazy_dotenv_settings,
|
lazy_dotenv_settings,
|
||||||
lazy_config_file_settings, # resolves/creates config file path
|
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:
|
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,
|
The data is added to the runtime settings, which have priority over the environment and
|
||||||
and finally merges the validated settings into the current instance. None values
|
the EOS configuration file. All configuration sources are re-evaluated afterwards.
|
||||||
are not merged.
|
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
data (dict): Dictionary containing field values to merge into the
|
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)
|
config.merge_settings_from_dict(new_data)
|
||||||
|
|
||||||
"""
|
"""
|
||||||
merged = merge_models(
|
previous_settings = ConfigEOS._runtime_settings
|
||||||
self,
|
ConfigEOS._runtime_settings = deep_merge(previous_settings, deepcopy(data))
|
||||||
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:
|
def reset_settings(self) -> None:
|
||||||
"""Reset all changed settings to environment/config file defaults.
|
"""Reset all changed settings to environment/config file defaults.
|
||||||
|
|
||||||
This functions basically deletes the settings provided before.
|
This functions basically deletes the settings provided before.
|
||||||
"""
|
"""
|
||||||
|
ConfigEOS._runtime_settings = {}
|
||||||
self._setup()
|
self._setup()
|
||||||
|
|
||||||
def revert_settings(self, backup_id: str) -> None:
|
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_data: dict[str, Any] = json.load(f)
|
||||||
backup_settings = migrate_config_data(backup_data)
|
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]]:
|
def list_backups(self) -> dict[str, dict[str, Any]]:
|
||||||
"""List available configuration backup files and extract metadata.
|
"""List available configuration backup files and extract metadata.
|
||||||
@@ -1094,11 +1149,11 @@ class ConfigEOS(SingletonMixin, SettingsEOSDefaults):
|
|||||||
"""Updates all configuration fields.
|
"""Updates all configuration fields.
|
||||||
|
|
||||||
This method updates all configuration fields using the following order for value retrieval:
|
This method updates all configuration fields using the following order for value retrieval:
|
||||||
1. Current settings.
|
1. Runtime settings.
|
||||||
2. Environment variables.
|
2. Environment variables.
|
||||||
3. EOS configuration file.
|
3. EOS configuration file.
|
||||||
4. Field default constants.
|
4. Field default constants.
|
||||||
|
|
||||||
The first non None value in priority order is taken.
|
The first non None value in priority order is taken.
|
||||||
"""
|
"""
|
||||||
self._setup(**self.model_dump())
|
self._setup()
|
||||||
|
|||||||
@@ -65,6 +65,37 @@ from akkudoktoreos.utils.datetimeutil import (
|
|||||||
_model_private_state: "weakref.WeakKeyDictionary[Union[PydanticBaseModel, PydanticModelNestedValueMixin], Dict[str, Any]]" = weakref.WeakKeyDictionary()
|
_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]:
|
def merge_models(source: BaseModel, update_dict: dict[str, Any]) -> dict[str, Any]:
|
||||||
"""Merge a Pydantic model instance with an update dictionary.
|
"""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.
|
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(
|
source_dict = source.model_dump(
|
||||||
exclude_unset=True,
|
exclude_unset=True,
|
||||||
exclude_computed_fields=True,
|
exclude_computed_fields=True,
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
import json
|
||||||
import tempfile
|
import tempfile
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import Any, Optional, Union
|
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
|
config_eos.merge_settings_from_dict({}) # No changes
|
||||||
|
|
||||||
assert config_eos.general.latitude == original_latitude # Should remain unchanged
|
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
|
||||||
|
|||||||
Reference in New Issue
Block a user