feat(config,cli): add assets-layout tooling and gridpool enterprise ownership - #130
Conversation
Reuse the platform-wide Frequenz API credentials when the Assets-specific `ASSETS_API_AUTH_KEY` and `ASSETS_API_SIGN_SECRET` are unset. Signed-off-by: cwasicki <126617870+cwasicki@users.noreply.github.com>
Validate config files offline via `AssetsConfig.load_from_files`, exiting non-zero on the first error so a config repo can gate PRs. Each file is checked on its own first, so a partial record that only completes after a merge is rejected: every committed file must name its own key and required fields. The merged pass then adds the cross-record checks. Signed-off-by: cwasicki <126617870+cwasicki@users.noreply.github.com>
Emit `assets.microgrids.<id>` entries and stamp `assets.version` so the generated file loads back without a migration. Signed-off-by: cwasicki <126617870+cwasicki@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
A confirmed crash path exists when the Assets API returns enterprise_id=None, and the release notes still include templated placeholder content.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR extends the assets config layout and gridpool-cli tooling, adding explicit gridpool enterprise ownership to the config model, enriching Assets-API-derived configs with enterprise_id, and rounding out CLI support for generating/patching/validating configs in the new assets.* layout.
Changes:
- Add
assets.gridpoolssupport (GridpoolConfig) and enterprise ownership lookup/validation viaAssetsConfig.find_enterprise()+ whole-document consistency checks. - Update config generation/dumping/patching to use
assets.versionand theassets.microgrids.*dotted-key layout, including stricter refusal of legacy layouts for in-place patching. - Add CLI subcommands
validateandfind-enterprise, plus credential fallbacks (FREQUENZ_API_{KEY,SECRET}) and in-place patch safety checks.
File summaries
| File | Description |
|---|---|
| tests/test_patch_config.py | Updates/expands patching tests for assets.microgrids.*, fill_missing, and legacy-layout refusal. |
| tests/test_load.py | Verifies enterprise_id is included when loading microgrids from the Assets API. |
| tests/test_dump_config.py | Updates dump/round-trip expectations for assets.version and assets.microgrids.* layout. |
| tests/test_config.py | Adds tests for gridpool enterprise ownership declaration/inference and invariant enforcement. |
| tests/test_cli.py | Adds CLI tests for credential fallback, in-place patch safety, find-enterprise, and validate. |
| src/frequenz/gridpool/config/_load.py | Adds enterprise_id population when building configs from Assets API metadata. |
| src/frequenz/gridpool/config/_gridpool.py | Introduces GridpoolConfig datamodel (gridpool_id, enterprise_id). |
| src/frequenz/gridpool/config/_assets.py | Adds gridpools, enterprise inference/consistency checks, and find_enterprise(). |
| src/frequenz/gridpool/config/init.py | Exposes GridpoolConfig in the public config package API. |
| src/frequenz/gridpool/cli/_patch_config.py | Adds overwrite-vs-fill-missing behavior, rejects legacy layouts, and patches assets.microgrids.*. |
| src/frequenz/gridpool/cli/_dump_config.py | Adds assets.version and emits assets.microgrids.* dotted keys. |
| src/frequenz/gridpool/cli/main.py | Adds validate + find-enterprise, credential fallback resolution, and safer --inplace writing. |
| RELEASE_NOTES.md | Updates release notes for new CLI/config features (but still contains template placeholders). |
| README.md | Updates CLI docs for new subcommands and credential fallback environment variables. |
Review details
Suppressed comments (1)
RELEASE_NOTES.md:35
- The “Bug Fixes” section is still a template placeholder comment. If there are no notable fixes, consider removing the placeholder and leaving the section empty, or add the relevant entries for this release.
<!-- Here goes notable bug fixes that are worth a special mention or explanation -->
- Files reviewed: 14/14 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
daniel-zullo-frequenz
left a comment
There was a problem hiding this comment.
I have a couple of questions but I'm not really familiar with this repo so someone else should have a look and approve it.
In-place patching overwrites the managed leaves by default; pass `--fill-missing` to only add the ones the file lacks. Signed-off-by: cwasicki <126617870+cwasicki@users.noreply.github.com>
In-place patching navigates the current `assets.microgrids` layout, so a top-level or `meta`-nested file would be duplicated rather than edited. Refuse it with a clear message pointing at a full rebuild. Signed-off-by: cwasicki <126617870+cwasicki@users.noreply.github.com>
A gridpool always belongs to one enterprise, so `assets.gridpools.<id>` names its `enterprise_id`. `AssetsConfig.enterprise_of` looks the owner up. Each entry must name its enterprise and be filed under its own `gridpool_id`. Signed-off-by: cwasicki <126617870+cwasicki@users.noreply.github.com>
`gridpool-cli find-enterprise <gridpool_id> <files>` prints the enterprise that owns the gridpool, read from the merged config. Exits non-zero when no `gridpools` entry names it. Signed-off-by: cwasicki <126617870+cwasicki@users.noreply.github.com>
`get_microgrid` already carries the owning enterprise, so a config built from the API now fills `MicrogridConfig.enterprise_id` instead of leaving it unset. Signed-off-by: cwasicki <126617870+cwasicki@users.noreply.github.com>
`check` now enforces the one-enterprise-per-gridpool invariant: a gridpool's microgrids may not disagree on the enterprise, and a declared `gridpools` enterprise must match the one its microgrids imply. Signed-off-by: cwasicki <126617870+cwasicki@users.noreply.github.com>
Signed-off-by: cwasicki <126617870+cwasicki@users.noreply.github.com>
cwasicki
left a comment
There was a problem hiding this comment.
Updated, also changed the enterprise ID handling.
Rounds out
gridpool-clifor theassetsconfig layout and adds gridpool enterprise ownership to the config model. The CLI can now generate, validate, patch, and query merged config files.Enterprise ownership: adds
GridpoolConfigentries underassets.gridpools.AssetsConfig.find_enterprise()returns the declared owner or infers it from related microgrids. Whole-document validation rejects conflicting enterprise assignments.Assets API enterprise metadata: configs derived from the Assets API now include each microgrid's
enterprise_id.find-enterprisecommand:gridpool-cli find-enterprise <gridpool_id> <files>prints the enterprise owning a gridpool from a merged config stack and exits non-zero when it cannot be determined.Credential fallbacks: accepts
FREQUENZ_API_KEY/FREQUENZ_API_SECRETwhen the Assets-specific credentials are unset. Credential pairs are never mixed between sources.validatecommand: validates each config file independently and then validates the merged stack, enabling config-repository CI checks.Current config output:
generate-configwrites theassets.microgridslayout and stampsassets.version.Safe in-place updates:
generate-config --inplacerefreshes managed values while preserving formatting.--fill-missingonly adds absent values, generated patches are validated before writing, and legacy layouts are rejected.