Skip to content

Update pyTibber to 0.27.0#86940

Merged
balloob merged 5 commits into
home-assistant:devfrom
tibber:update_tibber_lib
Mar 2, 2023
Merged

Update pyTibber to 0.27.0#86940
balloob merged 5 commits into
home-assistant:devfrom
tibber:update_tibber_lib

Conversation

@toini
Copy link
Copy Markdown
Contributor

@toini toini commented Jan 30, 2023

Breaking change

Glitre grid prices are removed as sensor attribute

Proposed change

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

This PR upgrades pyTibber to a version that dynamically queries web socket server URL effectively switching to new Tibber web socket server. More info at https://developer.tibber.com/.

Change log: Danielhiversen/pyTibber@0.26.8...0.27.0

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

@toini toini requested a review from Danielhiversen as a code owner January 30, 2023 11:19
@home-assistant home-assistant Bot added cla-needed dependency Pull requests marked as a dependency upgrade dependency-bump Pull requests that update a dependency file integration: tibber small-pr PRs with less than 30 lines. labels Jan 30, 2023
@home-assistant
Copy link
Copy Markdown
Contributor

Hi toini

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@home-assistant
Copy link
Copy Markdown
Contributor

Hey there @Danielhiversen, mind taking a look at this pull request as it has been labeled with an integration (tibber) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of tibber can trigger bot actions by commenting:

  • @home-assistant close Closes the issue.
  • @home-assistant rename Awesome new title Change the title of the issue.
  • @home-assistant reopen Reopen the issue.
  • @home-assistant unassign tibber Removes the current integration label and assignees on the issue, add the integration domain after the command.

@MartinHjelmare
Copy link
Copy Markdown
Member

Please add a link to a changelog or GitHub commit compare view for the version bump in the PR description. Thanks!

@MartinHjelmare
Copy link
Copy Markdown
Member

Please sign the CLA.

@Danielhiversen
Copy link
Copy Markdown
Member

Thanks

There are some breaking changes in the latest version of pyTibber that needs to be addressed: https://github.com/Danielhiversen/pyTibber/releases/tag/0.27.0

You should also fill out the checklist in this pull request. Most important The code change is tested and works locally.

You also need to sign the CLA. Please do so here.

@toini
Copy link
Copy Markdown
Contributor Author

toini commented Jan 30, 2023

@ztamas83 any chance you would be able to run this locally (I dont have Home Assistant set up myself) so we can tick off The code change is tested and works locally?

@toini
Copy link
Copy Markdown
Contributor Author

toini commented Jan 30, 2023

@MartinHjelmare @Danielhiversen CLA signed, change log for pyTibber added and I've requested @ztamas83 to test this locally. TODO: addressing the breaking changes.

@ztamas83
Copy link
Copy Markdown
Contributor

@ztamas83 any chance you would be able to run this locally (I dont have Home Assistant set up myself) so we can tick off The code change is tested and works locally?

Run in a local dev container, Tibber connected and live data received with my own credentials.

2023-01-30 19:53:57.211 DEBUG (MainThread) [tibber] Using websocket subscription url wss://websocket-api.tibber.com/v1-beta/gql/subscriptions

@toini
Copy link
Copy Markdown
Contributor Author

toini commented Feb 2, 2023

Updated The code change is tested and works locally to true.

@toini
Copy link
Copy Markdown
Contributor Author

toini commented Feb 2, 2023

@Danielhiversen do you know of any blockers for merge due to the breaking changes listed in pyTibber?

@frenck
Copy link
Copy Markdown
Member

frenck commented Feb 3, 2023

There is a merge conflict, can you please take a look?

I've marked this PR in the mean time. Please un-draft it once it is ready for review again by clicking the "Ready for review" button.

Thanks! 👍

../Frenck

@frenck frenck marked this pull request as draft February 3, 2023 16:12
@toini toini marked this pull request as ready for review February 13, 2023 07:30
@frenck frenck added the smash Indicator this PR is close to finish for merging or closing label Feb 13, 2023
@MartinHjelmare
Copy link
Copy Markdown
Member

Converting to draft until the breaking changes either have been addressed or confirmed not affecting the integration.

@MartinHjelmare MartinHjelmare marked this pull request as draft February 13, 2023 11:45
@Danielhiversen
Copy link
Copy Markdown
Member

The new exceptions should be handled here

await tibber_connection.update_info()

FatalHttpException -> return False
RetryableHttpException -> raise ConfigEntryNotReady

Then this is no longer needed:

if not tibber_connection.name:
raise ConfigEntryNotReady("Could not fetch Tibber data.")

Same here:

await tibber_connection.update_info()

FatalHttpException -> cannot_connect error
RetryableHttpException -> should probably give a new error message

@toini
Copy link
Copy Markdown
Contributor Author

toini commented Feb 22, 2023

@Danielhiversen would you be happy to adjust the code accordingly? You do have access.

@Danielhiversen
Copy link
Copy Markdown
Member

I don't have time to prioritize it in the short term.

@balloob
Copy link
Copy Markdown
Member

balloob commented Mar 1, 2023

Since your org forbids me to push commits, @toini, add this commit. It's untested.

commit b7c1617b418b718f20eb839c9137f8464985ce51
Author: Paulus Schoutsen <balloob@gmail.com>
Date:   Wed Mar 1 11:27:54 2023 -0500

    Handle new exceptions

diff --git a/homeassistant/components/tibber/__init__.py b/homeassistant/components/tibber/__init__.py
index 4d9c056068..6bd68e17c4 100644
--- a/homeassistant/components/tibber/__init__.py
+++ b/homeassistant/components/tibber/__init__.py
@@ -53,17 +53,18 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
 
     try:
         await tibber_connection.update_info()
-        if not tibber_connection.name:
-            raise ConfigEntryNotReady("Could not fetch Tibber data.")
 
-    except asyncio.TimeoutError as err:
-        raise ConfigEntryNotReady from err
-    except aiohttp.ClientError as err:
-        _LOGGER.error("Error connecting to Tibber: %s ", err)
-        return False
+    except (
+        asyncio.TimeoutError,
+        aiohttp.ClientError,
+        tibber.RetryableHttpException,
+    ) as err:
+        raise ConfigEntryNotReady("Unable to connect") from err
     except tibber.InvalidLogin as exp:
         _LOGGER.error("Failed to login. %s", exp)
         return False
+    except tibber.FatalHttpException:
+        return False
 
     await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)
 
diff --git a/homeassistant/components/tibber/config_flow.py b/homeassistant/components/tibber/config_flow.py
index d0adc0391a..b5cb4486cc 100644
--- a/homeassistant/components/tibber/config_flow.py
+++ b/homeassistant/components/tibber/config_flow.py
@@ -44,10 +44,14 @@ class TibberConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
                 await tibber_connection.update_info()
             except asyncio.TimeoutError:
                 errors[CONF_ACCESS_TOKEN] = "timeout"
-            except aiohttp.ClientError:
-                errors[CONF_ACCESS_TOKEN] = "cannot_connect"
             except tibber.InvalidLogin:
                 errors[CONF_ACCESS_TOKEN] = "invalid_access_token"
+            except (
+                aiohttp.ClientError,
+                tibber.RetryableHttpException,
+                tibber.FatalHttpException,
+            ):
+                errors[CONF_ACCESS_TOKEN] = "cannot_connect"
 
             if errors:
                 return self.async_show_form(
diff --git a/homeassistant/components/tibber/sensor.py b/homeassistant/components/tibber/sensor.py
index 2fe9e290d0..51bb9cda9a 100644
--- a/homeassistant/components/tibber/sensor.py
+++ b/homeassistant/components/tibber/sensor.py
@@ -44,6 +44,7 @@ from homeassistant.helpers.entity_registry import async_get as async_get_entity_
 from homeassistant.helpers.update_coordinator import (
     CoordinatorEntity,
     DataUpdateCoordinator,
+    UpdateFailed,
 )
 from homeassistant.util import Throttle, dt as dt_util
 
@@ -560,6 +561,8 @@ class TibberRtDataCoordinator(DataUpdateCoordinator):
 class TibberDataCoordinator(DataUpdateCoordinator[None]):
     """Handle Tibber data and insert statistics."""
 
+    config_entry: ConfigEntry
+
     def __init__(self, hass: HomeAssistant, tibber_connection: tibber.Tibber) -> None:
         """Initialize the data handler."""
         super().__init__(
@@ -572,9 +575,17 @@ class TibberDataCoordinator(DataUpdateCoordinator[None]):
 
     async def _async_update_data(self) -> None:
         """Update data via API."""
-        await self._tibber_connection.fetch_consumption_data_active_homes()
-        await self._tibber_connection.fetch_production_data_active_homes()
-        await self._insert_statistics()
+        try:
+            await self._tibber_connection.fetch_consumption_data_active_homes()
+            await self._tibber_connection.fetch_production_data_active_homes()
+            await self._insert_statistics()
+        except tibber.RetryableHttpException as err:
+            raise UpdateFailed(f"Error communicating with API ({err.status})") from err
+        except tibber.FatalHttpException:
+            # Fatal error. Reload config entry to show correct error.
+            self.hass.async_create_task(
+                self.hass.config_entries.async_reload(self.config_entry.entry_id)
+            )
 
     async def _insert_statistics(self) -> None:
         """Insert Tibber statistics."""

@robinostlund
Copy link
Copy Markdown
Contributor

robinostlund commented Mar 1, 2023

I have tried @balloob suggestion and it works with realtime data from pulse 😄
You are a saver @balloob

@CommanderROR9

This comment was marked as off-topic.

@odechr

This comment was marked as off-topic.

@ismarslomic

This comment was marked as off-topic.

@dieugab

This comment was marked as off-topic.

@ismarslomic

This comment was marked as off-topic.

@Danielhiversen
Copy link
Copy Markdown
Member

@toini
Would it be possible to also unblock 2023.3.* until this is merged?

#86940 (comment)

@simonepittis

This comment was marked as off-topic.

@ismarslomic

This comment was marked as off-topic.

@frenck
Copy link
Copy Markdown
Member

frenck commented Mar 2, 2023

⚠️ Before you respond to this PR

This is a PR, which is meant to review the contents of the code change in this PR. We are aware that this PR is fixing a connection issue.

There is no need to comment with a workaround, note you want this PR merged, or comment expressing the hope it ends up in the next patch.

PR comments are purely meant to review and work on the code change. Please use our community forums for other discussions.

Thanks! 👍

../Frenck

@toini
Copy link
Copy Markdown
Contributor Author

toini commented Mar 2, 2023

@balloob tried to patch just now but i got hunk at line 44 did not apply

@toini
Copy link
Copy Markdown
Contributor Author

toini commented Mar 2, 2023

patch applied, @balloob maybe you can double check?

@toini
Copy link
Copy Markdown
Contributor Author

toini commented Mar 2, 2023

@Danielhiversen can we merge this first. I guess we are not far away anymore?

@toini toini marked this pull request as ready for review March 2, 2023 10:22
@balloob balloob added this to the 2023.3.1 milestone Mar 2, 2023
@balloob balloob merged commit f69aa7a into home-assistant:dev Mar 2, 2023
@toini toini deleted the update_tibber_lib branch March 2, 2023 15:19
errors[CONF_ACCESS_TOKEN] = "cannot_connect"
except tibber.InvalidLogin:
errors[CONF_ACCESS_TOKEN] = "invalid_access_token"
except (
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We normally require 100% coverage on config flows. Could tests be added?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have near to 0 experience on python/unit testing on python and thus i've been strugling with this repo we try to help migrate to the new websocket endpoint. All assistance is welcome! @epenet.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I will have a look, shall I just create a new PR when done?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes please - and add a comment here with the link to the new PR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

#89088 adds unit tests for the config flow errors

balloob pushed a commit that referenced this pull request Mar 2, 2023
* Update pyTibber to 0.27.0

* Handle new exceptions
@balloob balloob mentioned this pull request Mar 2, 2023
@ztamas83 ztamas83 mentioned this pull request Mar 3, 2023
19 tasks
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

breaking-change cherry-picked cla-signed dependency Pull requests marked as a dependency upgrade dependency-bump Pull requests that update a dependency file integration: tibber Quality Scale: silver small-pr PRs with less than 30 lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.