-
-
Notifications
You must be signed in to change notification settings - Fork 2
Finalize 1.9.3 changelog with Retry-After hardening details #69
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 |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
|
|
||
| import asyncio | ||
| import logging | ||
| import math | ||
| import random | ||
| from typing import Any | ||
|
|
||
|
|
@@ -40,12 +41,15 @@ def _parse_retry_after(self, retry_after_raw: str) -> float: | |
| """Translate a Retry-After header into a delay in seconds.""" | ||
|
|
||
| try: | ||
| return float(retry_after_raw) | ||
| parsed = float(retry_after_raw) | ||
| if math.isfinite(parsed) and parsed > 0: | ||
| return parsed | ||
| return 2.0 | ||
|
Comment on lines
43
to
+47
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The References
|
||
| except (TypeError, ValueError): | ||
| retry_at = dt_util.parse_http_date(retry_after_raw) | ||
| if retry_at is not None: | ||
| delay = (retry_at - dt_util.utcnow()).total_seconds() | ||
| if delay > 0: | ||
| if math.isfinite(delay) and delay > 0: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding References
|
||
| return delay | ||
|
|
||
| return 2.0 | ||
|
|
@@ -118,7 +122,8 @@ async def async_fetch_pollen_data( | |
| delay = 2.0 | ||
| if retry_after_raw: | ||
| delay = self._parse_retry_after(retry_after_raw) | ||
| delay = min(delay, 5.0) + random.uniform(0.0, 0.4) | ||
| delay = delay + random.uniform(0.0, 0.4) | ||
| delay = max(0.0, min(delay, 5.0)) | ||
|
Comment on lines
+125
to
+126
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The clamping logic for the retry delay has been improved by applying References
|
||
| _LOGGER.warning( | ||
| "Pollen API 429 — retrying in %.2fs (attempt %d/%d)", | ||
| delay, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,22 +48,15 @@ def _rgb_from_api(color: dict[str, Any] | None) -> tuple[int, int, int] | None: | |
| """Build an (R, G, B) tuple from API color dict. | ||
|
|
||
| Rules: | ||
| - If color is not a dict, or an empty dict, or has no numeric channels at all, | ||
| return None (meaning "no color provided by API"). | ||
| - If color is not a dict, or an empty dict, return None | ||
| (meaning "no color provided by API"). | ||
| - If only some channels are present, missing ones are treated as 0 (black baseline) | ||
| but ONLY when at least one channel exists. This preserves partial colors like | ||
| {green, blue} without inventing a color for {}. | ||
| """ | ||
| if not isinstance(color, dict) or not color: | ||
| return None | ||
|
|
||
| # Check if any of the channels is actually provided as numeric | ||
| has_any_channel = any( | ||
| isinstance(color.get(k), (int, float)) for k in ("red", "green", "blue") | ||
| ) | ||
| if not has_any_channel: | ||
| return None | ||
|
|
||
| r = _normalize_channel(color.get("red")) | ||
| g = _normalize_channel(color.get("green")) | ||
| b = _normalize_channel(color.get("blue")) | ||
|
Comment on lines
60
to
62
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1093,6 +1093,106 @@ def test_plant_forecast_matches_codes_case_insensitively() -> None: | |
| assert entry["tomorrow_value"] == 4 | ||
|
|
||
|
|
||
| def test_coordinator_accepts_numeric_string_color_channels() -> None: | ||
| """Numeric string channels should be normalized into RGB/hex values.""" | ||
|
|
||
| payload = { | ||
| "dailyInfo": [ | ||
| { | ||
| "date": {"year": 2025, "month": 7, "day": 1}, | ||
| "pollenTypeInfo": [ | ||
| { | ||
| "code": "GRASS", | ||
| "displayName": "Grass", | ||
| "indexInfo": { | ||
| "value": 1, | ||
| "category": "LOW", | ||
| "color": {"red": "1", "green": "0", "blue": "0"}, | ||
| }, | ||
| } | ||
| ], | ||
| } | ||
| ] | ||
| } | ||
|
|
||
| fake_session = FakeSession(payload) | ||
| client = client_mod.GooglePollenApiClient(fake_session, "test") | ||
|
|
||
| loop = asyncio.new_event_loop() | ||
| hass = DummyHass(loop) | ||
| coordinator = coordinator_mod.PollenDataUpdateCoordinator( | ||
| hass=hass, | ||
| api_key="test", | ||
| lat=1.0, | ||
| lon=2.0, | ||
| hours=12, | ||
| language=None, | ||
| entry_id="entry", | ||
| forecast_days=1, | ||
| create_d1=False, | ||
| create_d2=False, | ||
| client=client, | ||
| ) | ||
|
|
||
| try: | ||
| data = loop.run_until_complete(coordinator._async_update_data()) | ||
| finally: | ||
| loop.close() | ||
|
|
||
| assert data["type_grass"]["color_hex"] == "#FF0000" | ||
| assert data["type_grass"]["color_rgb"] == [255, 0, 0] | ||
|
Comment on lines
+1096
to
+1143
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This new test case References
|
||
|
|
||
|
|
||
| def test_coordinator_ignores_invalid_string_color_channels() -> None: | ||
| """Non-numeric string channels should not emit RGB/hex values.""" | ||
|
|
||
| payload = { | ||
| "dailyInfo": [ | ||
| { | ||
| "date": {"year": 2025, "month": 7, "day": 1}, | ||
| "pollenTypeInfo": [ | ||
| { | ||
| "code": "GRASS", | ||
| "displayName": "Grass", | ||
| "indexInfo": { | ||
| "value": 1, | ||
| "category": "LOW", | ||
| "color": {"red": "foo"}, | ||
| }, | ||
| } | ||
| ], | ||
| } | ||
| ] | ||
| } | ||
|
|
||
| fake_session = FakeSession(payload) | ||
| client = client_mod.GooglePollenApiClient(fake_session, "test") | ||
|
|
||
| loop = asyncio.new_event_loop() | ||
| hass = DummyHass(loop) | ||
| coordinator = coordinator_mod.PollenDataUpdateCoordinator( | ||
| hass=hass, | ||
| api_key="test", | ||
| lat=1.0, | ||
| lon=2.0, | ||
| hours=12, | ||
| language=None, | ||
| entry_id="entry", | ||
| forecast_days=1, | ||
| create_d1=False, | ||
| create_d2=False, | ||
| client=client, | ||
| ) | ||
|
|
||
| try: | ||
| data = loop.run_until_complete(coordinator._async_update_data()) | ||
| finally: | ||
| loop.close() | ||
|
|
||
| assert data["type_grass"]["color_hex"] is None | ||
| assert data["type_grass"]["color_rgb"] is None | ||
|
Comment on lines
+1146
to
+1193
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The References
|
||
|
|
||
|
|
||
| def test_coordinator_ignores_nonfinite_color_channels() -> None: | ||
| """Non-finite color channel values should not crash or emit invalid colors.""" | ||
|
|
||
|
|
@@ -1460,6 +1560,81 @@ async def _fast_sleep(delay: float) -> None: | |
| assert delays == [5.0] | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ("retry_after", "now"), | ||
| [ | ||
| ("-10", None), | ||
| ("nan", None), | ||
| ("inf", None), | ||
| ( | ||
| "Wed, 10 Dec 2025 12:00:00 GMT", | ||
| datetime.datetime(2025, 12, 10, 12, 0, 5, tzinfo=datetime.UTC), | ||
| ), | ||
| ], | ||
| ) | ||
| def test_coordinator_retry_after_invalid_values_use_safe_default( | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| retry_after: str, | ||
| now: datetime.datetime | None, | ||
| ) -> None: | ||
| """Invalid Retry-After values should fall back to a safe finite delay.""" | ||
|
|
||
| session = SequenceSession( | ||
| [ | ||
| ResponseSpec( | ||
| status=429, | ||
| payload={"error": {"message": "Quota exceeded"}}, | ||
| headers={"Retry-After": retry_after}, | ||
| ), | ||
| ResponseSpec( | ||
| status=429, | ||
| payload={"error": {"message": "Quota exceeded"}}, | ||
| headers={"Retry-After": retry_after}, | ||
| ), | ||
| ] | ||
| ) | ||
| delays: list[float] = [] | ||
|
|
||
| async def _fast_sleep(delay: float) -> None: | ||
| assert isinstance(delay, float) | ||
| assert delay == delay | ||
| assert delay != float("inf") | ||
| assert delay != float("-inf") | ||
| delays.append(delay) | ||
|
|
||
| monkeypatch.setattr(client_mod.asyncio, "sleep", _fast_sleep) | ||
| monkeypatch.setattr(client_mod.random, "uniform", lambda *_args, **_kwargs: 0.0) | ||
| if now is not None: | ||
| monkeypatch.setattr(client_mod.dt_util, "utcnow", lambda: now) | ||
|
|
||
| client = client_mod.GooglePollenApiClient(session, "test") | ||
|
|
||
| loop = asyncio.new_event_loop() | ||
| hass = DummyHass(loop) | ||
| coordinator = coordinator_mod.PollenDataUpdateCoordinator( | ||
| hass=hass, | ||
| api_key="test", | ||
| lat=1.0, | ||
| lon=2.0, | ||
| hours=12, | ||
| language=None, | ||
| entry_id="entry", | ||
| forecast_days=1, | ||
| create_d1=False, | ||
| create_d2=False, | ||
| client=client, | ||
| ) | ||
|
|
||
| try: | ||
| with pytest.raises(client_mod.UpdateFailed, match="Quota exceeded"): | ||
| loop.run_until_complete(coordinator._async_update_data()) | ||
| finally: | ||
| loop.close() | ||
|
|
||
| assert session.calls == 2 | ||
| assert delays == [2.0] | ||
|
Comment on lines
+1563
to
+1635
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The new parameterized test References
|
||
|
|
||
|
|
||
| def test_coordinator_retries_then_raises_on_server_errors( | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| ) -> None: | ||
|
|
||
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.
The new
parsed > 0check treatsRetry-After: 0as invalid and forces the 2-second fallback, but a zero delay is a valid throttling response meaning "retry immediately". In environments where the upstream sends0during transient rate limits, this adds an unnecessary fixed delay on every retry and slows recovery compared to the previous behavior.Useful? React with 👍 / 👎.