Skip to content
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

supervisor-2021.02.5 has made configuration difficult #346

Closed
mdegat01 opened this issue Feb 9, 2021 · 16 comments
Closed

supervisor-2021.02.5 has made configuration difficult #346

mdegat01 opened this issue Feb 9, 2021 · 16 comments

Comments

@mdegat01
Copy link

mdegat01 commented Feb 9, 2021

supervisor-2021.02.5 seems to have significantly changed the add-on configuration experience. It now presents all configuration options as a generated form unless the configuration includes lists (since it can't handle that). This now causes this add-on to display an absolutely massive list of configuration options, nearly all of which are already configurable in the add-on UI.

It also doesn't seem to be parsing booleans correctly? For instance I see this in the configuration UI for the add-on:
Screen Shot 2021-02-08 at 10 44 57 PM
But as you can see below, I have both the sensors turned on in the add-on UI and can confirm they definitely appears in my list of entities:
Screen Shot 2021-02-08 at 10 46 14 PM
Screen Shot 2021-02-08 at 10 47 45 PM

Anyway I really don't know the implications of this change. I'm honestly afraid to modify anything in the configuration panel now because I don't really trust that its correctly pulling in the options I configured in the UI and I don't want to mess up my working configuration. I just wanted to make sure you were aware this happened as it now looks quite overwhelming for new users. And yea if existing users have to change any of the options not exposed in the UI it may be quite difficult now.

@sabeechen
Copy link
Owner

Oh good lord, this is terrible.

The reason some values appear different between the addon setting UI and the supervisor UI is that the supervisor UI interprets "not present" as false while the addon interprets not present as "default", which may be true or false depending on the option. For example enable_snapshot_stale_sensor is defaulted to true when not specified, but expose_extra_server is defaulted to false.

So setting a config option in the supervisor's editor should still work fine, you just might be setting it to the value it already was. I'd still use the addon's editor though, I'm not sure what the supervisor

I'm going to need to do some digging to see what this breaks, or if I can somehow opt out of this change for the addon. I haven't noticed problems on my instance after updating, but definitely the config UI in the supervisor is terrible now.

@mdegat01
Copy link
Author

mdegat01 commented Feb 9, 2021

Well I did notice that any of my addons with a list in their configuration didn't change at all. So if there's not an explicit opt-out option then a temporary fix might be to introduce a config option that accepts an array just to force it back to YAML form. Although I imagine eventually that will be converted to UI as well.

And yea everything works fine through the update but changing things might have negative consequences. Like what you said about booleans completely makes sense but now I wonder what else it's doing that to. Like if I go to the configuration options and I say "edit in YAML" here's what it shows me:

max_snapshots_in_hassio: 12
max_snapshots_in_google_drive: 12
days_between_snapshots: 1
use_ssl: true
snapshot_time_of_day: '04:30'
snapshot_password: '< password >'
snapshot_name: '{type} Snapshot {version_ha} {date} -MariaDB'
notify_for_stale_snapshots: false
send_error_reports: true
generational_days: 4
generational_weeks: 4
generational_months: 3
alternate_dns_servers: '94.140.14.14,94.140.15.15'
exclude_addons: core_mariadb
background_color: '#141414'
accent_color: '#2980b9'
certfile: ''
keyfile: ''
generational_day_of_week: ''
exclude_folders: ''
stop_addons: ''
drive_ipv4: ''
hassio_url: ''
home_assistant_url: ''
hassio_header: ''
drive_url: ''
authenticate_url: ''
refresh_url: ''
drive_refresh_url: ''
drive_token_url: ''
drive_authorize_url: ''
choose_folder_url: ''
error_report_url: ''
drive_host_name: ''
save_drive_creds_path: ''
stop_addon_state_path: ''
folder_file_path: ''
config_file_path: ''
retained_file_path: ''
secrets_file_path: ''
id_file_path: ''
backup_directory_path: ''
credentials_file_path: ''
ingress_token_file_path: ''
default_drive_client_id: ''
default_drive_client_secret: ''
drive_picker_api_key: ''
server_project_id: ''

I didn't touch any of the config options set to '' so if I save like this will it actually be ok? Or for these will it go from "config option not set mode" to assuming I did set those config values and set it to '' for some reason?

I don't actually need to set anything in here right now so this isn't an urgent issue for me. I just noticed one of my other addons changed so I started looking around to see what else happened. I was a little confused why it says use_ssl: true since I use Ingress but nothing seems to be broken so I am planning to ignore that for now. I mostly just figured you would want to know and didn't see another open issue on the topic.

@sabeechen
Copy link
Owner

I tired saving through the supervisor UI and it gave me an unparseable truncated error message:

Failed to save addon configuration, not a valid value for dictionary value @ data['options']. Got {'max_snapshots_in_hassio': 4, 'max_snapshots_in_google_drive': 4, 'days_between_snapshots': 1, 'use_ssl': False, 'snapshot_time_of_day': '11:37', 'specify_snapshot_folder': True, 'send_error_reports': False, 'certfile': '', 'keyfile': '', 'snapshot_name': '', 'generational_day_of_week': '', 'snapshot_password': '', 'exclude_folders': '', 'exclude_addons': '', 'stop_addons': '', 'drive_ipv4': '', 'alternate_dns_servers': '', 'background_color': '', 'accent_color': '', 'hassio_url': '', 'home_...

without any output from the supervisor logs. I'm guessing this is because the empty string isn't a valid value for some of the esoteric config options in the addon.

I think this is a well intentioned but ultimately broken supervisor feature. Unfortunately, I find out about this the same way you guys do, by installing the supervisor and seeing what goes wrong. I'm going to go see if someone in the supervisor dev world will hear my complaints. For now it looks like the config section is just broken for my addon.

@mdegat01
Copy link
Author

mdegat01 commented Feb 9, 2021

Yea understood.

Actually it seems like there might be a bunch of add-ons in a similar situation. When researching I dug up this issue. Apparently in addition to putting all config options in the face of users no matter how long the list is the latest supervisor release also introduced a breaking change by making schema required in options.json. This flat out breaks a number of add-ons including room-assistant.

Would be nice if there was a beta program for supervisor or something. Some way of giving add-on developers a heads up about what's coming before its already here. I'm sure I'm preaching to the choir on that though given you are an add-on developer.

@frenck
Copy link
Contributor

frenck commented Feb 9, 2021

Would be nice if there was a beta program for supervisor or something.

There is, it has been in beta for weeks. There is even a JOIN BETA button in the Supervisor UI.

@sabeechen
Copy link
Owner

Now that I've got you here @frenck, is there a way I can disable this change for this addon? I haven't figured out why but its unable to save config and if it did it looks like it would add empty strings for all the optional config parameters (which would break many, many things). I'm not sure if you're the right person to ask but it looks like you might have been involved in its development. I'm probably about to get an onslaught of bugs about this.

@frenck
Copy link
Contributor

frenck commented Feb 9, 2021

Provide a schema 🤷‍♂️

@sabeechen
Copy link
Owner

Is that not what this is in the addon's config?

@mdegat01
Copy link
Author

mdegat01 commented Feb 9, 2021

Would be nice if there was a beta program for supervisor or something.

There is, it has been in beta for weeks. There is even a JOIN BETA button in the Supervisor UI.

Oh, duh, how'd I miss that. Ignore me, sorry about that.

@frenck
Copy link
Contributor

frenck commented Feb 9, 2021

Sorry, from mobile, this missed a bit of context on your question and thus my reply was incorrect.

This addon provides a schema, which is great!

But yeah, with a metric ton of options, it might be a bit much. (Honestly, makes one wonder if all those options are needed and/or should be even par of the startup options, as most look like options from the application interface)

As for the empty string, yeah that is meh at best. Something I have not noticed personally, as I caught that with Bashio myself.

@sabeechen
Copy link
Owner

Some of them can and some of them can't, I use unset in the settings to indicate that a default should be used (defined in code). Until now that seemed to be a fine thing to do.

I'm not sure if what I'm seeing is a supervisor bug, working as intended, or because this addon is configured incorrectly. My plan for now is to isolate a simple repro and either file a supervisor bug or fix the addon if I can determine its doing something wrong. If you know which of those this is already please say because it would save me a lot of time.

@sabeechen
Copy link
Owner

In the meantime for anyone seeing this, its still possible to save configuration from within the supervisor UI. If you click the "..." and select "Edit in YAML" you can remove all of the empty configuration options, which allows it to save and appears to work as you would expect. All the empty values get added back in if you switch to the form UI editor, which is the default when you start to edit.

@sabeechen
Copy link
Owner

After doing some digging, it looks like the errors saving config in the supervisor's UI has to do with configuration options that validate as either a URL or regex. Unless you change it manually the UI attempts to save the empty string, which isn't a vaid URL and _may_not be a valid regex (depends on configuration). This addon has many such options with this problem.

This I think at the very least is a supervisor bug, because IIUC it makes it impossible to have a URL or regex configuration key be optional through the UI.

@sabeechen
Copy link
Owner

Well, I made my case. In the meantime I'll see if I can figure out a way to rewrite how this addon is configured without borking existing user's configuration. I suspect this addon is part of only a small minority that relied heavily on the mechanics of how the addon config editor used to work, so I couldn't really blame the frontend devs if they ignored it.

@sabeechen
Copy link
Owner

I just released v0.104.1 of the addon, which removes the configuration options that were making problems for the new config editor UI in HA. It still has the problem of diplaying False/Empty String/0 for values that aren't configured but at this point I don't think my complaint is going to get any traction so I think this is the best I can do.

To update the addon, go to:
Supervisor > Add-on Store > Click the ... in the top right > Refresh
and you should see the new version. HA will also find it automatically if you give it enough time.

Kindly re-open this issue if you continue to run into problems.

@mdegat01
Copy link
Author

mdegat01 commented Apr 22, 2021

Looks great! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants