-
Notifications
You must be signed in to change notification settings - Fork 747
Allow deeply nested dicts and lists in addon config schemas #6171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow deeply nested dicts and lists in addon config schemas #6171
Conversation
🔍 Existing Issues For ReviewYour pull request is modifying functions with the following pre-existing issues: 📄 File: supervisor/addons/options.py
Did you find this useful? React with a 👍 or 👎 |
|
The build issue on i386 is not related to this PR, but GitHub Container Registry returning |
|
Thanks for your PR!
True, but starting to support nested options with the current situation will create even more complex YAML configurations which people struggle to work with (e.g. see home-assistant/addons#4076). The current configuration frontend is custom for add-ons. There is also the idea to make the config option compatible with Home Assistant Core Selectors (https://www.home-assistant.io/docs/blueprint/selectors/). This would allow to leverage existing frontend code. |
Reusing the Selectors infrastructure is a great idea, but would solve only a part of the problem - there's a reason why integrations have the whole
The complex configuration schemas are mostly born out of a necessity defined by the problem domain, so there's no way to avoid it in some cases.
There's currently no way to adequately map certain configuration patterns that have their valid uses:
The underlying form toolkit being used already supports the first and second point (via As an aside, wouldn't it be possible to display a warning in the UI when something like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes arbitrary restrictions on how lists and dictionaries can be nested within addon configuration schemas, allowing for more complex and flexible schema definitions. The change maintains backward compatibility while enabling deeper nesting patterns that were previously disallowed.
Key changes:
- Updated schema validation to allow recursive nesting of dicts and lists (except list-of-lists)
- Refactored validation logic to use a unified
_validate_elementmethod for all types - Added comprehensive tests for complex nested schema scenarios
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| supervisor/addons/validate.py | Updated SCHEMA_ELEMENT to support recursive nesting with vol.Self references |
| supervisor/addons/options.py | Refactored validation methods to use unified _validate_element approach |
| tests/addons/test_options.py | Added test for complex dict/list nested validation scenarios |
| tests/addons/test_config.py | Added comprehensive schema validation tests covering various nesting patterns |
True. But you could make the argument first moving to selectors infrastructure before adding more complexity to the code might make sense.
Yeah that was my point, the lack of UI support causing problems today even with a single level of nesting. But given you've proposing to add proper UI support for nested (and deeply nested) configurations with options with home-assistant/frontend#26997, I am all for supporting deep nesting. Also the PRs all look quite straight forward without too much additional complexity. So I am all for moving forward with your PRs.
Nice 👍
You mean a generic error, whenever a nested dict with the same key as the key is detected 🤔 I guess we could do this, but potentially some add-on actually requires such a nesting. Anyhow, I think with the UI support for nesting that won't be necessary 🎉 . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UI debate aside, this honestly feels like more of a bug fix then a new feature. I kind of doubt banning dictionaries in dictionaries was intentional. This looks a lot cleaner and more sensible, LGTM 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you for your contribution 🙏 !
Proposed change
Remove the arbitrary (and undocumented) restrictions on how lists and dicts can be nested within themselves and each other in addon config schemas, so e.g. the following is now allowed:
The only remaining restriction is that list-of-lists (
field: [[str]]) are disallowed, because there's no way to represent them in the current encoding forUiOptions(and they can already be represented in a more self-documenting way:field: [{subfield: [str]}]).The current documentation at https://developers.home-assistant.io/docs/add-ons/configuration#options--schema does not mention the restrictions, and as such does not require an update per-se.
A documentation PR has been created at home-assistant/developers.home-assistant#2778, but can also be applied independently of the current PR.
Related issues
There is discussion at #2640 about changing the way addon configuration schemas are defined, but that discussion is already 4 years old. Given that the configuration UI already bails out to raw YAML syntax for some more complex schemas like the one down below, it does not worsen the current situation to simply loosen the current restrictions:
The only other currently available option for addons requiring more complex schemas is to forego Supervisor-side validation entirely.
Type of change
Additional information
Checklist
ruff format supervisor tests)If API endpoints or add-on configuration are added/changed:
CLI updated (if necessary)Not necessary, the JSON for the addon options that is exposed by the CLI is not changed.Client library updated (if necessary)Not necessary, the JSON for the addon options that is exposed by the client library is not changed.