Skip to content

Require config yaml and flow#289

Closed
MartinHjelmare wants to merge 2 commits into
masterfrom
adr-007-require-config-yaml-and-flow
Closed

Require config yaml and flow#289
MartinHjelmare wants to merge 2 commits into
masterfrom
adr-007-require-config-yaml-and-flow

Conversation

@MartinHjelmare
Copy link
Copy Markdown
Member

Closes #142, closes #143

Here's a related discussion:
home-assistant/core#26648 (comment)

@MartinHjelmare
Copy link
Copy Markdown
Member Author

CC @balloob and @Kane610

@Jc2k
Copy link
Copy Markdown
Member

Jc2k commented Sep 17, 2019

Are there exceptions to this? For example, some integrations have really good user experiences with config flow. Take my homekit_controller stuff - automatic discovery with zeroconf, enter the pin code, crypto keys are exchanged and stored. And I think Hue is similar with the "press button now" UI?

I am lucky as an existing integration maintainer because i'm "grandfathered in", but what about new integrations where there are rich UI based pairing experiences similar to these?

Copy link
Copy Markdown
Member

@pvizeli pvizeli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I agree that we should set the term to use domain config instead of platform config. At last there was some they used both and I suggest to use the domain config.

We require both config yaml and config flow support for all new integrations.

That means, user they relay on yaml have the hole life breaking changes and other they use integration are save from that. At this point I do not agree, that a integration need import yaml config with config flow.

YAML are nice to share stuff like automation or script, but for integration with setup API tokens or other things to going running is nothing what need yaml.

This is somethings that Paulus needs decide.

EDIT:
I also agree that we need now make an ADR that show how it should be in future because there are to many different ways around and that is not real userfriendly.

@@ -0,0 +1,64 @@
# 7. Require config yaml and flow
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# 7. Require config yaml and flow
# 7. Require config YAML and flow


This ADR has two faces that it wants to address:

1. The configuration structure disparity, and its consequences.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
1. The configuration structure disparity, and its consequences.
1. The configuration structure disparity and its consequences.


### Config structure disparity

There are currently many different ways of structuring an integration and its config in Home Assistant. We allow config yaml, either under the integration key, or under platform keys with the integration name. We allow a config flow with configuration entered via the GUI, stored in config entries. We allow importing config yaml to a config entry via config flow.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
There are currently many different ways of structuring an integration and its config in Home Assistant. We allow config yaml, either under the integration key, or under platform keys with the integration name. We allow a config flow with configuration entered via the GUI, stored in config entries. We allow importing config yaml to a config entry via config flow.
There are currently many different ways of structuring an integration and its config in Home Assistant. We allow config YAML, either under the integration key or under platform keys with the integration name. We allow a config flow with configuration entered via the GUI, stored in config entries. We allow importing config YAML to a config entry via config flow.


There are currently many different ways of structuring an integration and its config in Home Assistant. We allow config yaml, either under the integration key, or under platform keys with the integration name. We allow a config flow with configuration entered via the GUI, stored in config entries. We allow importing config yaml to a config entry via config flow.

These many options for configuration also impacts how the integration is structured and what Home Assistant backend APIs are used.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
These many options for configuration also impacts how the integration is structured and what Home Assistant backend APIs are used.
These many options for configuration also impact how the integration is structured and what Home Assistant backend APIs are used.


### Home Assistant's promise to users

On more than one occassion Home Assistant official representatives have said that config yaml will not go away as a way of configuring Home Assistant, even after introducing config flow. If we are serious about that promise, Home Assistant needs to take responsibility for the way that integrations offer ways of configuration. If we do not require integrations to offer yaml as a way of configuring, we have to change what we say to users and inform them that we can not promise that yaml will stay.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
On more than one occassion Home Assistant official representatives have said that config yaml will not go away as a way of configuring Home Assistant, even after introducing config flow. If we are serious about that promise, Home Assistant needs to take responsibility for the way that integrations offer ways of configuration. If we do not require integrations to offer yaml as a way of configuring, we have to change what we say to users and inform them that we can not promise that yaml will stay.
On more than one occasion Home Assistant official representatives have said that config YAML will not go away as a way of configuring Home Assistant, even after introducing config flow. If we are serious about that promise, Home Assistant needs to take responsibility for the way that integrations offer ways of configuration. If we do not require integrations to offer YAML as a way of configuring, we have to change what we say to users and inform them that we can not promise that YAML will stay.

## Proposal

1. We limit the configuration structure to one way for new integrations:
- We require all yaml config for an integration to be located under the integration domain key in config yaml, for all new integrations.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- We require all yaml config for an integration to be located under the integration domain key in config yaml, for all new integrations.
- We require all YAML config for an integration to be located under the integration domain key in config YAML, for all new integrations.


1. We limit the configuration structure to one way for new integrations:
- We require all yaml config for an integration to be located under the integration domain key in config yaml, for all new integrations.
- We require both config yaml and config flow support for all new integrations.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- We require both config yaml and config flow support for all new integrations.
- We require both config YAML and config flow support for all new integrations.

1. We limit the configuration structure to one way for new integrations:
- We require all yaml config for an integration to be located under the integration domain key in config yaml, for all new integrations.
- We require both config yaml and config flow support for all new integrations.
2. We do not allow removing support for either config yaml or config flow in existing integrations.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
2. We do not allow removing support for either config yaml or config flow in existing integrations.
2. We do not allow removing support for either config YAML or config flow in existing integrations.


1. The contributor work of implementing a single platform will become greater. Config flow requires unit tests, which not all contributors are familiar with.
2. New integration PRs will become longer and require more from code review.
3. Integration maintenance by developers will increase since they have to support both config yaml and config flow.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
3. Integration maintenance by developers will increase since they have to support both config yaml and config flow.
3. Integration maintenance by developers will increase since they have to support both config YAML and config flow.

@frenck
Copy link
Copy Markdown
Member

frenck commented Sep 17, 2019

I agree, partly. I think we should hold up to that promise, however, I've already noticed a couple of users in distress on not getting the difference, which I think need to be addressed.

  • A user that is doing both, config YAML & flow and getting lost in the process.
  • If a user did both, it is not clear which configuration is used. We do strive for YAML overriding flow, however, that is not always the case, nor is it visible.
  • No clear path of migration from YAML <-> Flow for the user.

For the above reasons alone, I did not add YAML support to the integrations I provided. (Even though, I am an advocate for writing my stuff up in YAML).

A suggestion, which is kinda out of scope for this ADR maybe, it that we should think about handling the flow (e.g., by disabling it) or some other visible indicator to show the user the settings are from YAML (e.g., like we do with the core config).

I also want to play a little devil's advocate: Why requiring config YAML and not the config flow? Seems unfair for users who want the config flow, right? A lot of feature requests are made against integrations that don't support it right now...

@cgarwood
Copy link
Copy Markdown
Member

Here's my 2 cents...

IMO the only reason to keep configuration.yaml around for things like integration/platform setup was to overcome the fact that you couldn't update a config entry's settings without removing it and setting it back up. In the case of zwave, that meant that if your usb path changed you either had to remove the zwave integration and re-set it up (thus getting a new network_key as well) or edit the usb_path in the .storage/core.config_entries file. So I added support in the zwave component so that a usb_path in configuration.yaml would override the config entry.

However, I'd love to see that go away and just be configured from the UI.

The big argument from the community for keeping YAML around is because people share configs and post it on GitHub. That's great, _but _ almost all the integration setup blocks in configuration.yaml are just specifying IP addresses/usernames/passwords in order to make the connection. There's zero value in that being shared on GitHub and nobody looking at configs on GitHub is looking for how to specify the connection details for deconz:, they're looking for things like scripts, automations, templates, etc (which are still YAML, and I've heard no talk of changing that).

The next big thing I hear is "but what about backups?" - back up your .storage folder along with everything else and you're set 🤷‍♂

"But I have a system set up where i commit config changes to git and they get linted and CI uploads them to my HA server and restarts it" - Cool, keep that for your automations, but use the much simpler UI config to set up connection information for an integration.

I see no reason to force integrations to maintain YAML config, especially now that we have config entry options and the settings in the config entry can be updated from the UI. It adds complexity to the component (especially since it's up to the component how it imports or merges those configs), is confusing for the users, and requires extra documentation.

@pvizeli
Copy link
Copy Markdown
Member

pvizeli commented Sep 17, 2019

I agree 100% with @frenck and that is why we should specify it and make it clear.

@dmulcahey
Copy link
Copy Markdown

I agree with @cgarwood and @Jc2k. I think the config flows are much more user friendly. I would think that approachability for the common non-developer user should be the focus in the future no? The UI driven config is much more natural for users IMHO. How many times have we had to help users who have been struggling with configuration because tabbing / spacing is off??? This isn’t a friendly experience for new users (especially non developers / non techies) and I would think that we would want to navigate away from this.

All of that said... couldn’t we build some core helper (config helper) / utility that automatically provides YAML support off of the config flow code for each integration? This way the integration devs build 1 but the platform provides both? This could potentially solve the ordering / precedence issue too. The api would allow you to query for configuration and it could handle checking YAML then the config entry)... just a thought.

@MartinHjelmare
Copy link
Copy Markdown
Member Author

Why requiring config YAML and not the config flow?

Maybe this wasn't clear, but this ADR does require all new integrations to support both config yaml and config flow.

@MartinHjelmare
Copy link
Copy Markdown
Member Author

MartinHjelmare commented Sep 17, 2019

Since this is a hot topic, I'll wait with changing any text until the discussion settles down and we have some consensus.

@Kane610
Copy link
Copy Markdown
Member

Kane610 commented Sep 17, 2019

I really don't like the aim for this ADR.

Just because configuration.yaml isn't going away isn't the same as saying an integration can't be config flow only. It is not a motivation to force it on all integrations.

Having this as a requirement will really stifle progress going forward. It would mean all new integrations need to start at least with configuration.yaml support and at a different stage the config entry support since previous recommendations have been to keep PRs as small as possible. If anything it will probably lead to slower convergence of using config flow and in the current state of things make less use of device registry and area registry.

Personally I'm moving away from configuration.yaml on my integrations, I don't see a reason to support both and I much prefer config flow over configuration.yaml.

@MartinHjelmare
Copy link
Copy Markdown
Member Author

MartinHjelmare commented Sep 17, 2019

To clarify, the aim of this ADR is this:

This ADR has two faces that it wants to address:

The configuration structure disparity, and its consequences.
What Home Assistant is promising users in terms of ways of configuration.

That aim and how we solve that is not the same thing. The current text reflects my proposal to reach that aim. If we come to a different solution that reaches the aim, the ADR will still be useful.

Edit: If I'd write the aim in action points I'd write this:

  • Use only one way to structure the config implementation of integrations.
  • Be clear to users what we can promise them regarding configuration.

@balloob
Copy link
Copy Markdown
Member

balloob commented Sep 17, 2019

I agree with all of you 😆

As this ADR has two parts, let me respond to both and I'll respond to some other comments along the way.

The configuration structure disparity, and its consequences.

It's very good to make a decision and write it down how we want things to be configured moving forward.

There are 4 different ways of doing YAML (1 top level with N configs, N top level with 1 config, 1 platform config with N configs, N platform configs with 1 config). By limiting future contributed YAML configurations to a single one, we will create a more predictable experience for YAML-based users.

If an integration supports both YAML and config entries, they should take one of either approaches:

  • It imports/syncs the YAML data to a config entry so that there is a single code path from there on (example: hue).
  • If the integration has no external dependencies (devices/services), it treats YAML as an immutable data store and uses a storage backend to support a mutable data store for a UI (example: person, lovelace)

I believe that YAML's true value lies in making it easy to version control and share your creations. This means YAML is great for integrations that work with Home Assistant data: Automations, Scripts, Scenes, Lovelace.

What Home Assistant is promising users in terms of ways of configuration.

Home Assistant is built up very modular. A lot of things are optional to implement. This makes it very easy to get an initial piece contributed. You don't have to implement unique ID, device info, etc. I don't think that having both YAML and config flow should be a requirement.

Integrations are allowed to have both, but it is up to the developer to decide how they want to spend their time. If other people want to contribute the missing configuration method (either YAML or config flow), they are free to do so.

@MartinHjelmare
Copy link
Copy Markdown
Member Author

MartinHjelmare commented Sep 18, 2019

If we don't require either only config yaml, only config flow or both config yaml and config flow, the disparity will continue and we won't reach the goal of this ADR.

Going back to only config yaml isn't an option. So that leaves two ways of reaching the goal, as I see it.

If an integration supports both YAML and config entries, they should take one of either approaches:

  • It imports/syncs the YAML data to a config entry so that there is a single code path from there on (example: hue).
  • If the integration has no external dependencies (devices/services), it treats YAML as an immutable data store and uses a storage backend to support a mutable data store for a UI (example: person, lovelace)

This is what we do today already. Just continuing that won't improve the situation much.

It's good to streamline the config yaml structure, but only doing that won't significantly decrease the ways we structure integrations and the backend APIs we recommend.

@MartinHjelmare
Copy link
Copy Markdown
Member Author

There's a third option that would allow us to decrease the disparity in the backend APIs usage: Require config flow and leave config yaml optional.

This will mean we always use the config entries API for new integrations.

@MartinHjelmare
Copy link
Copy Markdown
Member Author

Integrations are allowed to have both, but it is up to the developer to decide how they want to spend their time. If other people want to contribute the missing configuration method (either YAML or config flow), they are free to do so.

What should we do if one contributor wants to remove support for config yaml, does that, and then later another contributor comes and wants to add support back for config yaml?

@balloob
Copy link
Copy Markdown
Member

balloob commented Sep 18, 2019

I don't think that we should allow removing config yaml support. That would be an unnecessary breaking change.

I personally don't mind the contribution start with either config.yaml or config entries. Some people are more passionate about writing a driver/Python lib, others about the integration with HA.

I think that if we have to choose between a higher bar for contributions and a less predictable way how something would be configured, I would lean to less predictable.

For what it's worth, we could explore creating more config flow helpers. We have one based on discovery which is used a lot.

@pvizeli
Copy link
Copy Markdown
Member

pvizeli commented Sep 19, 2019

I agree with your statement above @balloob but with the second comment:

I don't think that we should allow removing config yaml support. That would be an unnecessary breaking change.

I have an issue. It's hard to maintain the breaking change faced YAML and the variable/dynamic config flow in one place. So I can understand the people they want "remove this hard in maintaining bridge" on integrations. We should not add stones on that if developers want to use the new config flow (what we want) and don't want to spend on the complexity to merge the old static config with stuff they can change over hass.io/UI/... in future.

@SeanPM5
Copy link
Copy Markdown

SeanPM5 commented Sep 19, 2019

Me personally, I like the idea of requiring config flow and making yaml optional. Then there would be clear, concise, and most importantly consistent instructions:

Go to the Integrations page and press the add button where you'll be guided through our easy setup process.

Currently the user experience is kind of all over the place:

  • Go to the Integrations page, sometimes, maybe? If you don't see it there and come up empty-handed, you gotta do it the other way I guess.
  • Even though these two integrations were added in the same exact version of Home Assistant, they require being set up in two radically different ways.
  • Sometimes integrations give you a choice between config flow or yaml, but often the config flow version lacks most of the available options that you can configure in YAML. So it's not really a true choice.
  • Straight up the vast majority of people are not willing to learn a markup language in order to install an app.
  • Even if someone does jump through all those hoops they still have the tedious aspect of visiting the website and searching through the integrations to find the documentation, referencing a long CONFIGURATION VARIABLES table and figuring everything out. And then possibly dealing with duplicate keys, bad indentation and numerous other ways things can go wrong.

I think that process is far too convoluted. You wouldn't ever be able to put YAML integrations in front of your wife or parents and have them figure it out. It doesn't pass the "normal person" test. They'll say "screw this" within two minutes and walk away.

I get wanting to make things easier for contributors. But for every contributor there's probably thousands of regular users that are struggling or giving up completely due to this aspect being too difficult. By not putting this extra burden on contributors it's instead making things harder than they need to be for the entire userbase.

Keep in mind that users can't even really begin writing automations or doing much of anything until they get over the initial hump of adding their integrations and devices.. A lot of things are getting easy now with Device automations and Lovelace Configure UI etc but the user can't even make it to that point without adding their devices first.

So this is a fairly crucial area to "get right" and make as easy as possible imo.

@balloob
Copy link
Copy Markdown
Member

balloob commented Sep 20, 2019

In that case we should make it dead easy to scaffold an integration with a config flow, including tests. I am going to see if I can cook up a generator.

@Adminiuga
Copy link
Copy Markdown

I believe that YAML's true value lies in making it easy to version control and share your creations.

Agreed, but in case when a configuration is just a "serial port path" then this value is diminished. (Side note: maybe allow config flows be exported/imported?)

If an integration supports both YAML and config entries, they should take one of either approaches:

  • It imports/syncs the YAML data to a config entry so that there is a single code path from there on (example: hue).

The problem with that approach: It is done only once and it is a common pitfall at least with ZHA when users update configuration.yaml but since it was already imported before, the new configuration.yaml doesn't have any effect. Very same problem mentioned by @cgarwood:

the fact that you couldn't update a config entry's settings without removing it and setting it back up. In the case of zwave, that meant that if your usb path changed you either had to remove the zwave integration and re-set it up (thus getting a new network_key as well) or edit the usb_path in the .storage/core.config_entries file.

so are there ways to update an existing entry without removing the integration completely? Cause in some cases that is not feasible as it removes all custom entity ids.

@balloob
Copy link
Copy Markdown
Member

balloob commented Sep 20, 2019

I guess the source=import that we have been using will indeed update it after the current entry has tried to set up, which is not ideal. That's a problem we could fix though.

I have just created a scaffolding script that should make it super easy to create a new integration with a config flow. It comes with tests includes. home-assistant/core#26777. With that in place, I think that we could consider making a config flow approach.

@rytilahti
Copy link
Copy Markdown
Member

rytilahti commented Sep 24, 2019

I have discussed my opinion about this previously on discord (and even done some wireframes to show how config forms could look like), so here's my $0.02: why not simply use the given configuration constraints the developers already have to define (in voluptuous) to create a Configuration (object) to be used by both ways of configuration?

How it would work:

  1. Each integration will have a configuration (even when a dummy one, if there are no options).
  2. Integration developers MUST define a schema for the configuration, if any. (we already have this with voluptuous)
  3. We map the schema to code and back (serialize, deserialize).
  4. A config flow would simply be an interface to fill the Configuration based on that schema. The developers can do whatever they wish during their flows and fill the configuration as they want, decoupling the presentation from the data.
  5. Schemas would be versioned, so a user could be presented with a flow / form / notification if and when they need to pay attention to potential configuration changes.
  6. As a bonus, if no config flow is given for the integration (or even if someone prefers a form entry over the given flow), we can still display a form based on the schema, including nice UI validators based on the schema (checkbox for boolean, radio button/dropdown for enums, validating IP address fields for addresses etc).

Now, those who want to manually manage their configuration manually could simply export a copy of the generated configuration (or prepare one manually by themselves) and adjust it as they want. The manually configured configuration would then take the precedence.

Also, the examples in documentation could be generated automatically using the schema (having example values), so that would be one less thing for doc maintainers to worry about.

Does this make sense? I think this way we could have no strict requirement for even every very simple integration to create a configuration flow (which would likely be some copy&paste boilerplate from some other integration..) a config flow while still offering both ways of configuration.

P.S. It may be worthwhile to take a look into how the (user-visible) configuration is done in WoT and other specifications, is there something we can adjust to our needs?

@robloh
Copy link
Copy Markdown

robloh commented Sep 25, 2019

As someone struggling to write an integration and of ease of system use & changes in the future I'm really liking @rytilahti proposal

@balloob
Copy link
Copy Markdown
Member

balloob commented Sep 25, 2019

Let's not derail the conversation with a proposal for something that is already in place with config flows, which work with the existing voluptuous schemas. You can open a new issue if you want to discuss this.

@ciotlosm
Copy link
Copy Markdown

I'm not sure if this is still being discussed, but I've been away for most of the "config flow" implementation and I thought my experience getting back into this would be helpful.

I really like config flows for all the benefits it brings with area registry and entity registry. Changing things without restarts speeds up the setup incredibly.

I used MQTT config flow integration renaming things as I was sitting close by on the fly (no restarts!). I was able to setup automation sin the UI from my mobile phone (!) while around the house setting up devices discovered on the fly through the mqtt integration. That was amazing to experience. Good luck doing that for bigger home setups without a laptop at hand when you need to tinker with yaml and home assistant restarts.

There are three worlds as I see:

  1. Users that want it simple and advanced users wanting it easy for friends they set this up for to avoid maintenance

  2. Advanced users that find the ability to use vi and git useful and cool

  3. Developers (usually advanced users) that want to contribute as easily as possible

  4. and 3. are the opposing side of the spectrum and config flow favors 1 while config yaml favors 3.

Reading through the comments it seems that balloob already built something to make config flows easier for 3.

I do think that for higher adoption of Home Assistant going with config flow as default is the best approach. This makes it even easier to maintain remote installation for home assistant for friends without special addons to reach the config yaml.

One thing that I find missing that would help a lot and will reduce confusion is a "simple" version of the Integrations page that just shows integration that have a config flow. Something similar to how the implementation of 'advanced' mode was added in the UI, but I guess this is separate to this discussion.

There are probably users that would just be happy to select backup of the configuration by integration and remove the anxiety that they no longer have the yaml at hand.

@MartinHjelmare
Copy link
Copy Markdown
Member Author

MartinHjelmare commented Oct 23, 2019

I think the current consensus is:

  1. Config yaml, if used, should be centralized under the integration domain. Platform yaml sections should be considered legacy for new integrations.
  2. Config yaml should be optional. The contributor, or code owner, decides if config yaml should be implemented.
  3. The promise to users is that for home assistant core config yaml will stay but for integrations it's up to each integration to support or not support config yaml.

Things that don't have full consensus or clear path forward:

  1. Should we require config flow? We now have scaffolding scripts for config flow 🚀. Import of config yaml and keeping the config entry updated when config yaml is changed is still not clearly generally solved.
  2. Should we allow removing config yaml from existing integration and if so how should that be decided? Is it up to the code owner? Should we require approval in an architecture issue or similar? Can we trust integrations to self manage this, or do we need more central control?

@MartinHjelmare
Copy link
Copy Markdown
Member Author

I'll close this now and open a new ADR PR for point 1 above where we have consensus.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Strategy for config methods Config structure for embedded platforms