Fixes an issue with OpenUV config import failing by bachya · Pull Request #17831 · home-assistant/core

bachya · 2018-10-26T20:18:16Z

Description:

Fixes an issue where OpenUV would fail in 0.81 when it was defined in configuration.yaml without specific latitude/longitude/elevation (i.e., relying on the default HASS latitude/longitude/elevation):

Config data is imported from configuration.yaml.
Default latitude/longitude/elevation doesn't populate properly.
Subsequent API calls fail.

In addition to fixing the bug, this PR makes it so that the latitude/longitude/elevation aren't stored in the config entry unless they are explicitly provided; if the HASS latitude/longitude/elevation ever changes, users shouldn't have to reconfigure this flow.

Until this is pushed out, the issue can be fixed by deleting and re-creating the OpenUV config entry (note that this will have to occur each time HASS restarts). @balloob Given this, would love to have this fix in 0.81.1.

Breaking Change: It's possible that users of OpenUV may have to delete and recreate the config entry.

Related issue (if applicable): fixes #17830

Pull request in home-assistant.io with documentation (if applicable): N/A

Example entry for `configuration.yaml` (if applicable):

openuv:
  api_key: !secret openuv_api_key

Checklist:

The code change is tested and works locally.
Local tests pass with tox. Your PR cannot be merged unless tests pass

bachya · 2018-10-26T22:32:35Z

@MartinHjelmare Would love your eyes on this given our conversation on the last one. Having the defaults as part of the config entry schema didn't cover a case, which caused this issue. Thoughts on my suggested route forward?

MartinHjelmare

What was the problem?

MartinHjelmare · 2018-10-27T07:33:26Z

homeassistant/components/openuv/config_flow.py

            return await self._show_form({CONF_API_KEY: 'invalid_api_key'})

+        if user_input.get(CONF_LATITUDE):
+            user_input[CONF_LATITUDE] = user_input[CONF_LATITUDE]


I don't understand this.

MartinHjelmare · 2018-10-27T07:35:31Z

homeassistant/components/openuv/__init__.py

+        CONF_SCAN_INTERVAL: conf[CONF_SCAN_INTERVAL],
+    }
+
+    if conf.get(CONF_LATITUDE):


if CONF_LATITUDE in conf:

houndci-bot

Some files could not be reviewed due to errors:

Traceback (most recent call last):

Traceback (most recent call last):
  File "/home/linters/.local/bin/flake8", line 7, in 
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'

houndci-bot

Some files could not be reviewed due to errors:

Traceback (most recent call last):

Traceback (most recent call last):
  File "/home/linters/.local/bin/flake8", line 7, in 
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'

balloob · 2018-10-27T10:07:49Z

I've addressed your comments, waiting for tests to pass.

bachya · 2018-10-27T17:06:56Z

@MartinHjelmare See #17830 for what the issue was.

@balloob Thanks for the assistance. Since tests are passing, I'm going to get this off our plate.

MartinHjelmare · 2018-10-27T17:11:15Z

It doesn't explain why the defaults in the form didn't work. Do you know?

bachya · 2018-10-27T17:14:20Z

The defaults didn’t work when you were importing configuration from configuration.yaml. If the openuv section of configuration.yaml didn’t specify a latitude and longitude, for example, the import step in the config flow didn’t attach the schema’s default (HASS latitude and longitude). The defaults only seemed to apply when somebody configured the config flow from scratch.

MartinHjelmare · 2018-10-27T17:23:10Z

Thanks! I see it now.

* Fixes an issue with OpenUV config import failing * Update * Update __init__.py * Update config_flow.py

exxamalte · 2018-10-28T11:59:34Z

I'm confused. My openuv configuration was working prior to 0.81.0/.1 as expected.
After I upgraded to 0.81.0 I had 2 entries for OpenUV in the "Integrations" section - one with the 4 entities I had before and one saying "None, None" without entities. Entities were updating once after a restart and then I saw failing updates, Status 500 errors, etc.
After I upgraded to 0.81.1 and without changing my configuration file, I was now welcomed with "New devices discovered" which turned out to be OpenUV. A click on "Configure" starts with asking me for my API key.
Does that mean that OpenUV does not support being configured via file anymore at all?

balloob · 2018-10-28T13:46:10Z

@exxamalte solved PRs are not the place for this.

Fixes an issue with OpenUV config import failing

fe5a2c5

bachya added this to the 0.81.1 milestone Oct 26, 2018

bachya self-assigned this Oct 26, 2018

homeassistant added cla-signed integration: openuv small-pr PRs with less than 30 lines. labels Oct 26, 2018

ghost added the in progress label Oct 26, 2018

bachya added the Hacktoberfest label Oct 26, 2018

bachya requested a review from MartinHjelmare October 26, 2018 22:31

bachya removed the request for review from MartinHjelmare October 27, 2018 01:13

bachya changed the title ~~Fixes an issue with OpenUV config import failing~~ WIP: Fixes an issue with OpenUV config import failing Oct 27, 2018

bachya mentioned this pull request Oct 27, 2018

OpenUV throws 500 errors in 0.81 #17830

Closed

Update

9338a79

bachya changed the title ~~WIP: Fixes an issue with OpenUV config import failing~~ Fixes an issue with OpenUV config import failing Oct 27, 2018

bachya requested a review from MartinHjelmare October 27, 2018 02:56

bachya added the breaking-change label Oct 27, 2018

MartinHjelmare reviewed Oct 27, 2018

View reviewed changes

Update __init__.py

39a5822

ghost assigned balloob Oct 27, 2018

houndci-bot reviewed Oct 27, 2018

View reviewed changes

Update config_flow.py

c26a627

houndci-bot reviewed Oct 27, 2018

View reviewed changes

bachya merged commit a22aad5 into home-assistant:dev Oct 27, 2018

ghost removed the in progress label Oct 27, 2018

balloob added the cherry-picked label Oct 27, 2018

balloob pushed a commit that referenced this pull request Oct 27, 2018

Fixes an issue with OpenUV config import failing (#17831)

edf2974

* Fixes an issue with OpenUV config import failing * Update * Update __init__.py * Update config_flow.py

balloob mentioned this pull request Oct 27, 2018

0.81.1 #17895

Merged

home-assistant locked as resolved and limited conversation to collaborators Oct 28, 2018

bachya deleted the fix-17830 branch October 28, 2018 16:02

Uh oh!

Conversation

bachya commented Oct 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Uh oh!

bachya commented Oct 26, 2018

Uh oh!

MartinHjelmare left a comment

Choose a reason for hiding this comment

Uh oh!

MartinHjelmare Oct 27, 2018

Choose a reason for hiding this comment

Uh oh!

MartinHjelmare Oct 27, 2018

Choose a reason for hiding this comment

Uh oh!

houndci-bot left a comment

Choose a reason for hiding this comment

Uh oh!

houndci-bot left a comment

Choose a reason for hiding this comment

Uh oh!

balloob commented Oct 27, 2018

Uh oh!

bachya commented Oct 27, 2018

Uh oh!

MartinHjelmare commented Oct 27, 2018

Uh oh!

bachya commented Oct 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MartinHjelmare commented Oct 27, 2018

Uh oh!

exxamalte commented Oct 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

balloob commented Oct 28, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

bachya commented Oct 26, 2018 •

edited

Loading

Example entry for `configuration.yaml` (if applicable):

bachya commented Oct 27, 2018 •

edited

Loading

exxamalte commented Oct 28, 2018 •

edited

Loading