-
-
Notifications
You must be signed in to change notification settings - Fork 37.6k
Opower: fall back to usage-only reads when cost endpoint fails #169224
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -563,8 +563,17 @@ def _update_with_finer_cost_reads( | |||||||||||||||
| account, AggregateType.DAY, start, end | ||||||||||||||||
| ) | ||||||||||||||||
| except ApiException as err: | ||||||||||||||||
| _LOGGER.error("Error getting daily cost reads: %s", err) | ||||||||||||||||
| raise | ||||||||||||||||
| _LOGGER.warning( | ||||||||||||||||
| "Error getting daily cost reads, falling back to usage-only: %s", | ||||||||||||||||
| err, | ||||||||||||||||
| ) | ||||||||||||||||
| try: | ||||||||||||||||
| daily_cost_reads = await self.api.async_get_cost_reads( | ||||||||||||||||
| account, AggregateType.DAY, start, end, usage_only=True | ||||||||||||||||
| ) | ||||||||||||||||
| except ApiException: | ||||||||||||||||
| _LOGGER.warning("Usage-only daily reads also failed, using monthly") | ||||||||||||||||
| return cost_reads | ||||||||||||||||
| _LOGGER.debug("Got %s daily cost reads", len(daily_cost_reads)) | ||||||||||||||||
| _update_with_finer_cost_reads(cost_reads, daily_cost_reads) | ||||||||||||||||
| if account.read_resolution == ReadResolution.DAY: | ||||||||||||||||
|
|
@@ -581,8 +590,19 @@ def _update_with_finer_cost_reads( | |||||||||||||||
| account, AggregateType.HOUR, start, end | ||||||||||||||||
| ) | ||||||||||||||||
| except ApiException as err: | ||||||||||||||||
| _LOGGER.error("Error getting hourly cost reads: %s", err) | ||||||||||||||||
| raise | ||||||||||||||||
| _LOGGER.warning( | ||||||||||||||||
| "Error getting hourly cost reads, falling back to usage-only: %s", | ||||||||||||||||
| err, | ||||||||||||||||
| ) | ||||||||||||||||
| try: | ||||||||||||||||
| hourly_cost_reads = await self.api.async_get_cost_reads( | ||||||||||||||||
| account, AggregateType.HOUR, start, end, usage_only=True | ||||||||||||||||
| ) | ||||||||||||||||
| except ApiException: | ||||||||||||||||
| _LOGGER.warning( | ||||||||||||||||
| "Usage-only hourly reads also failed, using coarser reads" | ||||||||||||||||
|
Comment on lines
+601
to
+603
|
||||||||||||||||
| except ApiException: | |
| _LOGGER.warning( | |
| "Usage-only hourly reads also failed, using coarser reads" | |
| except ApiException as err: | |
| _LOGGER.warning( | |
| "Usage-only hourly reads also failed, using coarser reads: %s", | |
| err, |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -250,8 +250,6 @@ async def test_coordinator_migration( | |||||||||||||||||||||||||||
| ("async_get_accounts", None), | ||||||||||||||||||||||||||||
| ("async_get_forecast", None), | ||||||||||||||||||||||||||||
| ("async_get_cost_reads", AggregateType.BILL), | ||||||||||||||||||||||||||||
| ("async_get_cost_reads", AggregateType.DAY), | ||||||||||||||||||||||||||||
| ("async_get_cost_reads", AggregateType.HOUR), | ||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||
| async def test_coordinator_api_exceptions( | ||||||||||||||||||||||||||||
|
|
@@ -290,6 +288,54 @@ async def side_effect(account, agg_type, start, end): | |||||||||||||||||||||||||||
| await coordinator._async_update_data() | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| @pytest.mark.parametrize( | ||||||||||||||||||||||||||||
| "failing_aggregate_type", | ||||||||||||||||||||||||||||
| [AggregateType.DAY, AggregateType.HOUR], | ||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||
| async def test_coordinator_cost_reads_fallback_on_api_error( | ||||||||||||||||||||||||||||
| recorder_mock: Recorder, | ||||||||||||||||||||||||||||
| hass: HomeAssistant, | ||||||||||||||||||||||||||||
| mock_config_entry: MockConfigEntry, | ||||||||||||||||||||||||||||
| mock_opower_api: AsyncMock, | ||||||||||||||||||||||||||||
| failing_aggregate_type: AggregateType, | ||||||||||||||||||||||||||||
| ) -> None: | ||||||||||||||||||||||||||||
| """Test that daily/hourly cost read failures fall back to usage-only reads.""" | ||||||||||||||||||||||||||||
| coordinator = OpowerCoordinator(hass, mock_config_entry) | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| # Use a single ELEC account with HOUR resolution so all read levels are attempted | ||||||||||||||||||||||||||||
| account = mock_opower_api.async_get_accounts.return_value[0] | ||||||||||||||||||||||||||||
| mock_opower_api.async_get_accounts.return_value = [account] | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| t1 = dt_util.as_utc(datetime(2023, 1, 1, 0)) | ||||||||||||||||||||||||||||
| t2 = dt_util.as_utc(datetime(2023, 1, 2, 0)) | ||||||||||||||||||||||||||||
| bill_read = CostRead( | ||||||||||||||||||||||||||||
| start_time=t1, end_time=t2, consumption=10.0, provided_cost=1.0 | ||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||
| usage_read = CostRead( | ||||||||||||||||||||||||||||
| start_time=t1, end_time=t2, consumption=10.0, provided_cost=0.0 | ||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| async def side_effect( | ||||||||||||||||||||||||||||
| acc: object, | ||||||||||||||||||||||||||||
| agg_type: AggregateType, | ||||||||||||||||||||||||||||
| start: object, | ||||||||||||||||||||||||||||
| end: object, | ||||||||||||||||||||||||||||
| usage_only: bool = False, | ||||||||||||||||||||||||||||
| ) -> list[CostRead]: | ||||||||||||||||||||||||||||
| if agg_type == failing_aggregate_type and not usage_only: | ||||||||||||||||||||||||||||
| raise ApiException(message="HTTP Error: 500", url="http://example.com") | ||||||||||||||||||||||||||||
| if usage_only: | ||||||||||||||||||||||||||||
| return [usage_read] | ||||||||||||||||||||||||||||
| return [bill_read] | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| mock_opower_api.async_get_cost_reads.side_effect = side_effect | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| # Should NOT raise — the coordinator should fall back to usage-only reads | ||||||||||||||||||||||||||||
| result = await coordinator._async_update_data() | ||||||||||||||||||||||||||||
|
loganrosen marked this conversation as resolved.
|
||||||||||||||||||||||||||||
| await async_wait_recording_done(hass) | ||||||||||||||||||||||||||||
| assert result is not None | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
Comment on lines
+337
to
+338
|
||||||||||||||||||||||||||||
| await_calls = mock_opower_api.async_get_cost_reads.await_args_list | |
| assert any( | |
| call.args[1] == failing_aggregate_type | |
| and not call.kwargs.get("usage_only", False) | |
| for call in await_calls | |
| ) | |
| assert any( | |
| call.args[1] == failing_aggregate_type | |
| and call.kwargs.get("usage_only") is True | |
| for call in await_calls | |
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Log the usage-only daily ApiException details (e.g., include the exception in the warning) so operators can see why both the primary and fallback calls failed.