Consolidate the config loaders around one raw-table merge (#127)

Collapses the config package's merge machinery onto a single raw-table
deep-merge and cleans up the loader API around it.

- Drop the object-merge helpers: `merge_microgrid_configs`,
`merge_config_maps` and the nested per-object merge are gone.
`load_configs` now assembles its layers (default files < Assets API <
override files) as raw assets tables, deep-merges
them, and loads once. The nested `assets.microgrids.<id>` structure
gives per-ID merge and unique-ID passthrough for free; a None-skip in
`_assets._deep_merge` (a no-op on real TOML, which has no null)
reproduces the old "an unset
    override keeps the base value" semantics for the API layer.
- Validate the merged whole. Because it loads once at the end,
`load_configs` validates the merged document instead of each layer
independently — so an override that is only complete after merging is
now legal (see also #126).
- load_configs returns the whole `AssetsConfig`. It already built the
full merged document internally, then discarded everything but
`.microgrids`, silently dropping the relations and `market_locations`
the file layers carry. It now returns
the document; callers wanting only the microgrid map read `.microgrids`.
- `load_configs_from_api` is now private (`_load_microgrids_from_api`).
An API-only load is just `load_configs(assets_client=...,
microgrid_ids=...).microgrids`, so the standalone loader no longer needs
to be public.

 ## Breaking changes (0.x)

  - `merge_config_maps` / `merge_microgrid_configs` removed.
- `load_configs` returns `AssetsConfig`, not `dict[str,
MicrogridConfig]`.
  - `load_configs_from_api` no longer public.

Author: cwasicki

RELEASE_NOTES.md

@@ -14,10 +14,12 @@
   configs = AssetsConfig.load_from_files(path).microgrids
   ```
 
-  `load_configs` keeps its existing interface. `load_from_files` layers files
-  field by field instead of replacing a complete microgrid entry; fields omitted
-  by a later file retain the value from the earlier layer and cannot be removed
-  by omission.
+  `load_configs` now returns the whole `AssetsConfig` rather than just its
+  microgrids, so the file layers' `relations` and `market_locations` survive the
+  merge; replace `load_configs(...)` with `load_configs(...).microgrids` where
+  only the microgrid map is needed. `load_from_files` layers files field by field
+  instead of replacing a complete microgrid entry; fields omitted by a later file
+  retain the value from the earlier layer and cannot be removed by omission.
 
 - `Metadata.delivery_area` is removed. Move its value to a relation's
   `delivery_area.code`, and set `delivery_area.code_type` when the code is not
@@ -30,6 +32,14 @@
   `AssetsConfig.load_from_files(files).microgrids`, or use the returned
   `AssetsConfig` directly to keep relations and market locations.
 
+- `load_configs_from_api` is now private. For an API-only load call
+  `load_configs(assets_client=..., microgrid_ids=...)` and read its `.microgrids`.
+
+- `merge_config_maps` and `merge_microgrid_configs` are removed. Layering is now
+  done on the raw tables before loading, inside `load_configs` and
+  `AssetsConfig.load_from_files`; pass all the layers to one of those instead of
+  merging loaded objects.
+
 - Relation validity bounds and `at` query instants must include a UTC offset.
 
 - The implementation modules `config.load` and `config.microgrid` are now

src/frequenz/gridpool/cli/__main__.py

@@ -210,24 +210,28 @@ async def generate_config(
     async with AssetsApiClient(url, auth_key=key, sign_secret=secret) as client:
         if inplace:
             # default_file is the patch target here, not a merge input.
-            configs = await load_configs(
-                assets_client=client,
-                override_files=override_file,
-                microgrid_ids=ids,
-                component_graph_config=_graph_config(
-                    prefer_meters_in_component_formulas
-                ),
-            )
+            configs = (
+                await load_configs(
+                    assets_client=client,
+                    override_files=override_file,
+                    microgrid_ids=ids,
+                    component_graph_config=_graph_config(
+                        prefer_meters_in_component_formulas
+                    ),
+                )
+            ).microgrids
         else:
-            configs = await load_configs(
-                default_files=default_file,
-                assets_client=client,
-                override_files=override_file,
-                microgrid_ids=ids,
-                component_graph_config=_graph_config(
-                    prefer_meters_in_component_formulas
-                ),
-            )
+            configs = (
+                await load_configs(
+                    default_files=default_file,
+                    assets_client=client,
+                    override_files=override_file,
+                    microgrid_ids=ids,
+                    component_graph_config=_graph_config(
+                        prefer_meters_in_component_formulas
+                    ),
+                )
+            ).microgrids
 
     if not configs:
         raise click.ClickException("No microgrids could be loaded; nothing to write.")

src/frequenz/gridpool/config/__init__.py

@@ -6,10 +6,7 @@
 from frequenz.microgrid_component_graph import ComponentGraphConfig, FormulaOverrides
 
 from ._assets import AssetsConfig
-from ._load import (
-    load_configs,
-    load_configs_from_api,
-)
+from ._load import load_configs
 from ._microgrid import (
     BatteryConfig,
     ComponentCategory,
@@ -19,8 +16,6 @@
     MicrogridConfig,
     PVConfig,
     WindConfig,
-    merge_config_maps,
-    merge_microgrid_configs,
 )
 from ._topology import (
     DeliveryAreaConfig,
@@ -46,7 +41,4 @@
     "ValidityConfig",
     "WindConfig",
     "load_configs",
-    "load_configs_from_api",
-    "merge_config_maps",
-    "merge_microgrid_configs",
 ]

src/frequenz/gridpool/config/_assets.py

@@ -27,16 +27,57 @@ def _deep_merge(base: dict[str, Any], override: dict[str, Any]) -> dict[str, Any
     Nested tables are merged recursively; any other value in `override` replaces
     the one in `base`. Merging the raw tables, before they are loaded, keeps a
     field left unset in an override from resetting the base value to its default.
+
+    A `None` override is skipped so the base value survives. TOML has no null, so
+    this only matters for a dumped-object layer (e.g. the Assets API) where unset
+    fields carry `None`; on real file tables it is a no-op.
     """
     result = dict(base)
     for key, value in override.items():
+        if value is None:
+            continue
         if isinstance(value, dict) and isinstance(result.get(key), dict):
             result[key] = _deep_merge(result[key], value)
         else:
             result[key] = value
     return result
 
 
+def _merge_file_tables(
+    config_files: str | Path | list[str | Path],
+) -> dict[str, Any]:
+    """Read and deep-merge the raw `assets` tables of one or more files.
+
+    Later files win, entry by entry. Paths that are not files are skipped with a
+    warning. Merging before loading lets an override leave a field unset without
+    resetting the base value.
+
+    Args:
+        config_files: A path or list of paths to TOML config files.
+
+    Returns:
+        The merged raw `assets` table, unvalidated.
+
+    Raises:
+        ValueError: If no config files are given.
+    """
+    if isinstance(config_files, (str, Path)):
+        paths = [Path(config_files)]
+    else:
+        paths = [Path(f) for f in config_files]
+    if not paths:
+        raise ValueError("No config files provided. Please provide at least one.")
+
+    merged: dict[str, Any] = {}
+    for config_path in paths:
+        if not config_path.is_file():
+            _logger.warning("Config path %s is not a file, skipping.", config_path)
+            continue
+        # pylint: disable-next=protected-access
+        merged = _deep_merge(merged, AssetsConfig._read_assets_table(config_path))
+    return merged
+
+
 @dataclass
 class AssetsConfig:
     """Entities described by a config document, keyed by their ID."""
@@ -386,24 +427,8 @@ def load_from_files(
 
         Returns:
             The merged document.
-
-        Raises:
-            ValueError: If no config files are given.
         """
-        if isinstance(config_files, (str, Path)):
-            paths = [Path(config_files)]
-        else:
-            paths = [Path(f) for f in config_files]
-        if not paths:
-            raise ValueError("No config files provided. Please provide at least one.")
-
-        merged: dict[str, Any] = {}
-        for config_path in paths:
-            if not config_path.is_file():
-                _logger.warning("Config path %s is not a file, skipping.", config_path)
-                continue
-            merged = _deep_merge(merged, cls._read_assets_table(config_path))
-
+        merged = _merge_file_tables(config_files)
         loaded = cls.Schema().load(merged)
         assert isinstance(loaded, cls)
         if check:

src/frequenz/gridpool/config/_load.py

@@ -5,6 +5,7 @@
 
 import logging
 from pathlib import Path
+from typing import Any
 
 from frequenz.client.assets import AssetsApiClient
 from frequenz.client.common.microgrid import MicrogridId
@@ -24,12 +25,11 @@
     pv_inverter_ids,
     pv_meter_ids,
 )
-from ._assets import AssetsConfig
+from ._assets import AssetsConfig, _deep_merge, _merge_file_tables
 from ._microgrid import (
     ComponentTypeConfig,
     Metadata,
     MicrogridConfig,
-    merge_config_maps,
 )
 
 _logger = logging.getLogger(__name__)
@@ -41,7 +41,7 @@ async def load_configs(
     override_files: str | Path | list[str | Path] | None = None,
     microgrid_ids: list[int] | None = None,
     component_graph_config: ComponentGraphConfig | None = None,
-) -> dict[str, "MicrogridConfig"]:
+) -> AssetsConfig:
     """Load configs from up to three sources and merge them in layers.
 
     Combines up to three sources, listed here from lowest to highest
@@ -81,14 +81,14 @@ async def load_configs(
             Requires an `assets_client`.
 
     Returns:
-        dict[str, MicrogridConfig]:
-            Mapping from microgrid ID (as string) to the merged
-            `MicrogridConfig` instance.
+        The merged document. Use `.microgrids` for just the microgrid map; the
+        file layers may also contribute `relations` and `market_locations`,
+        which the Assets API layer does not provide.
 
     Raises:
-        ValueError: If none of the three sources is provided, or if
-            `microgrid_ids` or `component_graph_config` is given without an
-            `assets_client`.
+        ValueError: If none of the three sources is provided, if `microgrid_ids`
+            or `component_graph_config` is given without an `assets_client`, or
+            if a file's `assets.microgrids` is not a table.
     """
     if default_files is None and assets_client is None and override_files is None:
         raise ValueError("At least one config source must be provided.")
@@ -99,28 +99,46 @@ async def load_configs(
     if component_graph_config is not None and assets_client is None:
         raise ValueError("component_graph_config requires an assets_client.")
 
-    configs: dict[str, MicrogridConfig] = {}
+    default_table: dict[str, Any] = {}
     if default_files is not None:
-        configs = AssetsConfig.load_from_files(default_files).microgrids
+        default_table = _merge_file_tables(default_files)
 
-    override_configs: dict[str, MicrogridConfig] = {}
+    override_table: dict[str, Any] = {}
     if override_files is not None:
-        override_configs = AssetsConfig.load_from_files(override_files).microgrids
+        override_table = _merge_file_tables(override_files)
 
+    merged = default_table
     if assets_client is not None:
         if microgrid_ids is None:
-            microgrid_ids = sorted({int(mid) for mid in (*configs, *override_configs)})
-        assets_configs = await load_configs_from_api(
+            file_ids: set[str] = set()
+            for table in (default_table, override_table):
+                microgrids = table.get("microgrids", {})
+                if not isinstance(microgrids, dict):
+                    raise ValueError(
+                        f"`assets.microgrids` must be a table, got {type(microgrids)}"
+                    )
+                file_ids |= set(microgrids)
+            microgrid_ids = sorted(int(mid) for mid in file_ids)
+        assets_configs = await _load_microgrids_from_api(
             assets_client=assets_client,
             microgrid_ids=microgrid_ids,
             component_graph_config=component_graph_config,
         )
-        configs = merge_config_maps(base=configs, override=assets_configs)
+        schema = MicrogridConfig.Schema()
+        api_table: dict[str, Any] = {
+            "microgrids": {mid: schema.dump(cfg) for mid, cfg in assets_configs.items()}
+        }
+        merged = _deep_merge(merged, api_table)
+
+    merged = _deep_merge(merged, override_table)
 
-    return merge_config_maps(base=configs, override=override_configs)
+    loaded = AssetsConfig.Schema().load(merged)
+    assert isinstance(loaded, AssetsConfig)
+    loaded.check()
+    return loaded
 
 
-async def load_configs_from_api(
+async def _load_microgrids_from_api(
     assets_client: AssetsApiClient,
     microgrid_ids: list[int],
     component_graph_config: ComponentGraphConfig | None = None,
@@ -129,8 +147,8 @@ async def load_configs_from_api(
 
     For each microgrid, fetches its location metadata (latitude, longitude) and
     then derives the per-type formulas and meter/inverter/component IDs from its
-    component graph. This is the canonical single-source loader so that callers
-    (e.g. the forecast pipeline) do not have to re-implement this logic.
+    component graph. Builds the Assets API layer that `load_configs` merges; for
+    an API-only load call `load_configs(assets_client=..., microgrid_ids=...)`.
 
     The two steps fail independently: a microgrid whose metadata cannot be
     fetched is skipped, while one whose component graph cannot be derived is

src/frequenz/gridpool/config/_microgrid.py

@@ -5,7 +5,6 @@
 
 import logging
 import re
-from copy import deepcopy
 from dataclasses import field
 from datetime import datetime
 from typing import Any, ClassVar, Literal, Self, Type, cast, get_args
@@ -331,66 +330,3 @@ def _load_table_entries(cls, data: dict[str, Any]) -> dict[str, Self]:
             mgrids[mid] = mgrid
 
         return mgrids
-
-
-def merge_microgrid_configs(
-    base: MicrogridConfig,
-    override: MicrogridConfig,
-) -> MicrogridConfig:
-    """Merge two `MicrogridConfig` objects.
-
-    The *override* config takes precedence over *base*.  Nested dictionaries
-    are merged recursively.  If a field in *override* is `None` the value
-    from *base* is retained, so partial overrides never nullify existing data.
-
-    Args:
-        base: The base MicrogridConfig.
-        override: The overriding MicrogridConfig.
-
-    Returns:
-        A new MicrogridConfig representing the merged result.
-    """
-    schema = MicrogridConfig.Schema()
-    base_dict = schema.dump(base)
-    override_dict = schema.dump(override)
-
-    def _deep_merge(a: dict[Any, Any], b: dict[Any, Any]) -> dict[Any, Any]:
-        result = deepcopy(a)
-        for k, v in b.items():
-            if v is None:
-                continue
-            if isinstance(v, dict) and isinstance(result.get(k), dict):
-                result[k] = _deep_merge(result[k], v)
-            else:
-                result[k] = v
-        return result
-
-    merged = schema.load(_deep_merge(base_dict, override_dict))
-    assert isinstance(merged, MicrogridConfig)
-    return merged
-
-
-def merge_config_maps(
-    base: dict[str, MicrogridConfig],
-    override: dict[str, MicrogridConfig],
-) -> dict[str, MicrogridConfig]:
-    """Merge two dictionaries of `MicrogridConfig` objects.
-
-    For microgrid IDs present in both maps the configs are merged via
-    `merge_microgrid_configs`.  IDs that exist only in one map are
-    included unchanged.
-
-    Args:
-        base: The base dictionary of MicrogridConfig objects.
-        override: The overriding dictionary of MicrogridConfig objects.
-
-    Returns:
-        A new dictionary representing the merged result.
-    """
-    merged = dict(base)
-    for mid, cfg in override.items():
-        if mid in merged:
-            merged[mid] = merge_microgrid_configs(merged[mid], cfg)
-        else:
-            merged[mid] = cfg
-    return merged

tests/test_config.py

@@ -237,7 +237,9 @@ async def test_merge_prefixed_base_with_legacy_override(tmp_path: Path) -> None:
         tmp_path, "override.toml", '1.meta.microgrid_id = 1\n1.meta.name = "Renamed"\n'
     )
 
-    configs = await load_configs(default_files=base, override_files=override)
+    configs = (
+        await load_configs(default_files=base, override_files=override)
+    ).microgrids
 
     assert configs["1"].meta.name == "Renamed"
     assert configs["1"].component_type_ids("pv") == [101, 102]