Join GitHub today
Split up yaml loaders into multiple files #23774
Some implementation details of
I intended to add a feature to the yaml loader to expand environment variables in strings (such as in docker-compose.yml). When considering this I noticed that the
The first improvement I would like to propose is contained in this PR and splits up the yaml code, following the structure in Ansible where the custom yaml loaders are split up into multiple files with clear roles.
A second step would be to no longer monkey patch
A (optional) third step would be to add "expand environment variables in strings" to make values (e.g. database connection strings) configurable using environment variables that are expanded using the common
If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
If the code does not interact with devices:
I think this kind of a tricky PR but think the code gets a lot more structured because of it. It is a purely structural change.
I realized that I did not check other test cases (i.e.
I'll follow that one, thanks for the heads up! I think that I caught all the places directly using
I was aware of that and think this work is orthogonal to that. If this restructuring (and the structure as in Ansible) is actually clearer to other developers.
Is there a way to add telemetry so we can see how often these special loaders are actually used? In my opinion some of them seem to have niche use cases.
I don't think we should expand environment variables in all strings in the config - we shouldn't change the value of any YAML object at all without the user intentionally requesting to do so with a YAML tag.
Reason is: There are probably many real use-cases (like shell script component) where they should not be expanded. If we would expand them, that might be a very un-intuitive annoyance. Plus it would be a bad breaking change. Plus I don't see why
Another option would be a system esphome uses: substitutions (https://esphome.io/guides/configuration-types.html#config-substitutions) - only keys that are explicitly defined by the user are expanded. Everything else that looks like a substitution but is not defined as one, is not expanded.
Problem is HA templates also use jinja2 - if we evaluate jinja2 at load time it would fundamentally mess with HA templating.
If you want to pursue it further, I would suggest opening an architecture issue (https://github.com/home-assistant/architecture).
May 9, 2019
13 of 14 checks passed
I was away for the weekend so this comes a bit late - but I'll expand on the environment variables.
Thanks for the quick feedback and the explanation of your considerations!
It is a product change I would definitely not consider before having access to telemetry. Changing semantics without a good reason is annoying. I would only consider this for single line strings and only if it conflicts in a limited number of cases.
The use case is the general "configure through environment variables" pattern that is the part of the 12 factor app pattern that I appreciate.
My use case is building a connection string to influxdb and mysql from the environment variables that docker compose creates. This needs multiple tokens (
That is the way. There are many paths to take (adding a (version) tag to the yaml, only applying to single line strings, only apply to some type of multiline quoting, only apply within quotes, etc). For now I hardcoded the connection string in my config