Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions homeassistant/components/iaqualink/coordinator.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import httpx
from iaqualink.exception import (
AqualinkServiceException,
AqualinkServiceThrottledException,
AqualinkServiceUnauthorizedException,
)

Expand Down Expand Up @@ -44,6 +45,12 @@ async def _async_update_data(self) -> None:
await self.system.update()
except AqualinkServiceUnauthorizedException as err:
raise ConfigEntryAuthFailed("Invalid credentials for iAquaLink") from err
except AqualinkServiceThrottledException:
_LOGGER.warning(
"Rate limited by iAquaLink system %s, will retry later",
self.system.serial,
)
return
Comment on lines +48 to +53
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.

Do you know when to retry later? You can use UpdateFailed and it's methods to postpone the next update

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.

Not really. I'm assuming it will simply retry at the next update_interval.

Right now the library handles 429 but I'm actually thinking of yanking the handling and put it on the client-side because it doesn't really make sense to have two layers doing retries (and I need to do it on the HA-side anyway).

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.

But isn’t the integration technically unavailable when you hit the rate limit? Like, while it’s rate limiting I would assume you also are unable to trigger any actions?

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.

Normally when a rate limit occurs this is the case. The retry_after also renders the integration as unavailable during that that time.
It's not a fully fledge backoff strategy with multiple retries, before rendering the integration in a retry state.

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.

@zweckj Potentially, that depends how the ratelimiting is done. I suspect the read path and the write path have different ratelimits but it hasn't been confirmed (and I can't test it myself since I don't have the hardware).

I saw habitica was doing something similar (stale data when getting ratelimited) so I figured there was a precedent.

except (AqualinkServiceException, httpx.HTTPError) as err:
raise UpdateFailed(
f"Unable to update iAquaLink system {self.system.serial}: {err}"
Expand Down
57 changes: 56 additions & 1 deletion tests/components/iaqualink/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from iaqualink.client import AqualinkClient
from iaqualink.exception import (
AqualinkServiceException,
AqualinkServiceThrottledException,
AqualinkServiceUnauthorizedException,
)
from iaqualink.systems.iaqua.device import (
Expand All @@ -16,6 +17,7 @@
IaquaThermostat,
)
from iaqualink.systems.iaqua.system import IaquaSystem
import pytest

from homeassistant.components.binary_sensor import DOMAIN as BINARY_SENSOR_DOMAIN
from homeassistant.components.climate import DOMAIN as CLIMATE_DOMAIN
Expand Down Expand Up @@ -46,7 +48,9 @@ async def _advance_coordinator_time(
hass: HomeAssistant, freezer: FrozenDateTimeFactory
) -> None:
"""Advance time to trigger coordinator update interval."""
freezer.tick(delta=UPDATE_INTERVAL_BY_SYSTEM_TYPE["iaqua"])
update_interval = UPDATE_INTERVAL_BY_SYSTEM_TYPE["iaqua"]

freezer.tick(delta=update_interval)
async_fire_time_changed(hass, dt_util.utcnow())
await hass.async_block_till_done(wait_background_tasks=True)

Expand Down Expand Up @@ -104,6 +108,57 @@ async def fail_update() -> None:
assert state.state == STATE_UNAVAILABLE


async def test_system_rate_limited_keeps_entities_available(
hass: HomeAssistant,
config_entry: MockConfigEntry,
client: AqualinkClient,
freezer: FrozenDateTimeFactory,
caplog: pytest.LogCaptureFixture,
) -> None:
"""Test a rate-limited update keeps entities at their last known state."""
config_entry.add_to_hass(hass)

system = get_aqualink_system(client, cls=IaquaSystem)
system.online = True
system.update = AsyncMock()
systems = {system.serial: system}
light = get_aqualink_device(
system, name="aux_1", cls=IaquaLightSwitch, data={"state": "1"}
)
devices = {light.name: light}
system.get_devices = AsyncMock(return_value=devices)

with (
patch(
"homeassistant.components.iaqualink.AqualinkClient.login",
return_value=None,
),
patch(
"homeassistant.components.iaqualink.AqualinkClient.get_systems",
return_value=systems,
),
):
await hass.config_entries.async_setup(config_entry.entry_id)
await hass.async_block_till_done()

entity_ids = hass.states.async_entity_ids(LIGHT_DOMAIN)
assert len(entity_ids) == 1
entity_id = entity_ids[0]

state = hass.states.get(entity_id)
assert state is not None
assert state.state == STATE_ON

system.update = AsyncMock(side_effect=AqualinkServiceThrottledException)

await _advance_coordinator_time(hass, freezer)

state = hass.states.get(entity_id)
assert state is not None
assert state.state == STATE_ON
assert "Rate limited by iAquaLink" in caplog.text


async def test_light_service_calls_update_entity_state(
hass: HomeAssistant,
config_entry: MockConfigEntry,
Expand Down