From 9dbbebcfb90b8580f74046e76c84616d421e651f Mon Sep 17 00:00:00 2001 From: cwasicki <126617870+cwasicki@users.noreply.github.com> Date: Mon, 17 Aug 2026 18:47:51 +0200 Subject: [PATCH] feat(config): read microgrid entries from `assets.microgrids` Microgrids are only the first entity these files describe; gridpools, market locations and the relations between them follow, and bare numeric top-level keys leave nowhere to put them. The namespace also makes the file mergeable with other config sources. A key like `23.meta.name` claims the top level outright and says nothing about what it identifies, so it can collide with unrelated settings and is unreadable as a deployment override. Under `assets` the generated inventory stays apart from the operator settings in `app.*`, and two sources writing the same key now genuinely mean the same microgrid. Top-level entries still load, with a deprecation warning, until the config files have migrated. A file carrying both layouts is rejected: it is a half-finished migration rather than a merge. Signed-off-by: cwasicki <126617870+cwasicki@users.noreply.github.com> --- RELEASE_NOTES.md | 13 +++- src/frequenz/gridpool/config/microgrid.py | 53 +++++++++++++-- tests/test_config.py | 82 ++++++++++++++++++++++- 3 files changed, 142 insertions(+), 6 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index d466455..fc8def2 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -6,7 +6,18 @@ ## Upgrading - +- Microgrid config files should nest their entries under `assets.microgrids`: + + ```toml + assets.microgrids.23.meta.name = "..." + ``` + + Bare microgrid IDs at the top level still load but log a deprecation warning, + and support for them will be removed. The namespace keeps generated inventory + data apart from operator settings under `app.*`, so a config file can carry + both without its keys colliding. + + A file mixing both layouts is rejected. ## New Features diff --git a/src/frequenz/gridpool/config/microgrid.py b/src/frequenz/gridpool/config/microgrid.py index faf43c7..b59c356 100644 --- a/src/frequenz/gridpool/config/microgrid.py +++ b/src/frequenz/gridpool/config/microgrid.py @@ -301,18 +301,18 @@ def _load_table_entries(cls, data: dict[str, Any]) -> dict[str, Self]: """Load microgrid configurations from table entries. Args: - data: The loaded TOML data. + data: The table mapping microgrid IDs to their entries. Returns: A dict mapping microgrid IDs to MicrogridConfig instances. Raises: - ValueError: If top-level keys are not numeric microgrid IDs + ValueError: If the keys are not numeric microgrid IDs or if there is a microgrid ID mismatch. TypeError: If microgrid data is not a dict. """ if not all(str(k).isdigit() for k in data.keys()): - raise ValueError("All top-level keys must be numeric microgrid IDs.") + raise ValueError("All microgrid keys must be numeric microgrid IDs.") mgrids = {} for mid, entry in data.items(): @@ -337,6 +337,51 @@ def _load_table_entries(cls, data: dict[str, Any]) -> dict[str, Self]: return mgrids + @classmethod + def _microgrid_table(cls, data: dict[str, Any], source: str) -> dict[str, Any]: + """Pick the microgrid entries out of a parsed config document. + + Entries live under `assets.microgrids`. A document without an `assets` + table is read in the deprecated layout, where the entries sit at the + top level. + + Args: + data: The parsed TOML document. + source: Name of the document, used in messages. + + Returns: + The table mapping microgrid IDs to their entries. + + Raises: + TypeError: If `assets` is not a table. + ValueError: If both layouts are present, which means a + half-migrated file rather than a merge. + """ + if "assets" not in data: + _logger.warning( + "%s: top-level microgrid IDs are deprecated, " + "nest the entries under `assets.microgrids` instead.", + source, + ) + return data + + assets = data["assets"] + if not isinstance(assets, dict): + raise TypeError(f"{source}: `assets` must be a table, got {type(assets)}") + + if unprefixed := sorted(k for k in data if k != "assets"): + raise ValueError( + f"{source}: keys {unprefixed} sit outside `assets` while the file " + "already has an `assets` table; move them under `assets.microgrids`." + ) + + microgrids = assets.get("microgrids", {}) + if not isinstance(microgrids, dict): + raise TypeError( + f"{source}: `assets.microgrids` must be a table, got {type(microgrids)}" + ) + return microgrids + @classmethod def load_from_file(cls, config_path: Path) -> dict[str, Self]: """ @@ -353,7 +398,7 @@ def load_from_file(cls, config_path: Path) -> dict[str, Self]: assert isinstance(data, dict) - return cls._load_table_entries(data) + return cls._load_table_entries(cls._microgrid_table(data, str(config_path))) def merge_microgrid_configs( diff --git a/tests/test_config.py b/tests/test_config.py index 184c761..8102a19 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -3,6 +3,7 @@ """Tests for the frequenz.lib.notebooks.config module.""" +import logging from pathlib import Path from typing import Any @@ -10,7 +11,11 @@ from pytest_mock import MockerFixture from frequenz.gridpool import MicrogridConfig -from frequenz.gridpool.config import ComponentTypeConfig, load_configs_from_files +from frequenz.gridpool.config import ( + ComponentTypeConfig, + load_configs, + load_configs_from_files, +) VALID_CONFIG: dict[str, dict[str, Any]] = { "1": { @@ -160,3 +165,78 @@ def _assert_optional_field(value: float | None, expected: float) -> None: if value is not None: if value != expected: raise AssertionError(f"Expected {expected}, got {value}") + + +_PREFIXED_TOML = """ +assets.microgrids.1.meta.microgrid_id = 1 +assets.microgrids.1.meta.name = "Test Grid" +assets.microgrids.1.ctype.pv.meter = [101, 102] +""" + +_LEGACY_TOML = """ +1.meta.microgrid_id = 1 +1.meta.name = "Test Grid" +1.ctype.pv.meter = [101, 102] +""" + + +def _write(tmp_path: Path, name: str, text: str) -> Path: + """Write `text` to `name` under `tmp_path` and return the path.""" + path = tmp_path / name + path.write_text(text) + return path + + +def test_load_prefixed(tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None: + """Entries under `assets.microgrids` load without a deprecation warning.""" + configs = MicrogridConfig.load_from_file( + _write(tmp_path, "prefixed.toml", _PREFIXED_TOML) + ) + + assert configs["1"].meta.name == "Test Grid" + assert configs["1"].component_type_ids("pv") == [101, 102] + assert "deprecated" not in caplog.text + + +def test_load_legacy_warns(tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None: + """Top-level entries still load, but warn and name the file.""" + path = _write(tmp_path, "legacy.toml", _LEGACY_TOML) + + with caplog.at_level(logging.WARNING): + configs = MicrogridConfig.load_from_file(path) + + assert configs["1"].meta.name == "Test Grid" + assert "deprecated" in caplog.text + assert str(path) in caplog.text + + +def test_load_mixed_layouts_rejected(tmp_path: Path) -> None: + """A half-migrated file with both layouts is an error, not a merge.""" + path = _write( + tmp_path, + "mixed.toml", + _PREFIXED_TOML + '2.meta.microgrid_id = 2\n2.meta.name = "Other Grid"\n', + ) + + with pytest.raises(ValueError, match="outside `assets`"): + MicrogridConfig.load_from_file(path) + + +def test_load_assets_without_microgrids(tmp_path: Path) -> None: + """An `assets` table without microgrids yields no configs and no warning.""" + path = _write(tmp_path, "other.toml", 'assets.gridpool.7.name = "GP"\n') + + assert not MicrogridConfig.load_from_file(path) + + +async def test_merge_prefixed_base_with_legacy_override(tmp_path: Path) -> None: + """Layers of either layout merge as usual, since layout is resolved on load.""" + base = _write(tmp_path, "base.toml", _PREFIXED_TOML) + override = _write( + tmp_path, "override.toml", '1.meta.microgrid_id = 1\n1.meta.name = "Renamed"\n' + ) + + configs = await load_configs(default_files=base, override_files=override) + + assert configs["1"].meta.name == "Renamed" + assert configs["1"].component_type_ids("pv") == [101, 102]