Update weatherkit to fetch hourly data for 7 days#164494
Update weatherkit to fetch hourly data for 7 days#164494zxdavb merged 8 commits intohome-assistant:devfrom
Conversation
While WeatherKit will return up to 10 days, I figured a week was fine. I want to be able to display hourly weather for the next couple of days, not just the next 24 hours.
|
Hey there @tjhorner, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Pull request overview
This PR extends the WeatherKit integration’s hourly forecast request window to cover the next 7 days (instead of the default shorter window), enabling longer-range hourly forecasts to be exposed via the existing coordinator-backed weather entity.
Changes:
- Add an explicit
hourly_start/hourly_endrange to the WeatherKitget_weather_datacall. - Use a UTC “now” timestamp to define the 7-day hourly forecast window.
Comments suppressed due to low confidence (3)
homeassistant/components/weatherkit/coordinator.py:77
- This change introduces new behavior (explicit hourly range and 7-day window) but there is no test asserting the
get_weather_datacall uses the expectedhourly_start/hourly_endvalues (and that they’re omitted when hourly forecasts aren’t supported). Adding a coordinator-level test that inspects the mocked call args would help prevent regressions.
now = datetime.now(tz=UTC)
updated_data = await self.client.get_weather_data(
self.config_entry.data[CONF_LATITUDE],
self.config_entry.data[CONF_LONGITUDE],
self.supported_data_sets,
hourly_start=now,
hourly_end=now + timedelta(days=7),
)
homeassistant/components/weatherkit/coordinator.py:77
- Requesting 7 days of hourly forecast data on every coordinator refresh can significantly increase response size and API usage (update interval is 5 minutes). Consider reducing the refresh frequency when
hourly_endis extended, or splitting current conditions vs forecast into separate refresh paths/coordinators so current weather can remain frequent while the 7‑day forecast updates less often.
now = datetime.now(tz=UTC)
updated_data = await self.client.get_weather_data(
self.config_entry.data[CONF_LATITUDE],
self.config_entry.data[CONF_LONGITUDE],
self.supported_data_sets,
hourly_start=now,
hourly_end=now + timedelta(days=7),
)
homeassistant/components/weatherkit/coordinator.py:77
hourly_start/hourly_endare passed even whensupported_data_setsdoes not includeDataSetType.HOURLY_FORECAST(e.g., locations where hourly is unavailable). It would be safer to only include these kwargs when requesting the hourly forecast dataset to avoid sending unused parameters and potential upstream validation errors.
updated_data = await self.client.get_weather_data(
self.config_entry.data[CONF_LATITUDE],
self.config_entry.data[CONF_LONGITUDE],
self.supported_data_sets,
hourly_start=now,
hourly_end=now + timedelta(days=7),
)
|
I think you have to make some comment as to the risk (or absence os such) of getting tagged for API abuse by the vendor... This will affect all users of this integration - they're going to be obligated to pull data that - you could reasonably argue - most people don't want... Maybe a better solution would be to provide an action to enable this functionality? Or have it in config flow? |
|
@zxdavb that's a totally fair point. That being said:
|
tjhorner
left a comment
There was a problem hiding this comment.
Seems reasonable to me.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
homeassistant/components/weatherkit/coordinator.py:77
- Fetching 7 days of hourly forecast data on every coordinator refresh (currently
update_interval=5minutes) will greatly increase response size and the size of the weather entity’sforecastattributes. This can increase bandwidth/API usage and significantly bloat state/recorder storage over time. Consider limiting the hourly range (for example, a smaller window such as 48–72 hours) and/or separating hourly forecast fetching into a less-frequent refresh cadence or cached update that is not re-downloaded every 5 minutes.
now = datetime.now(tz=UTC)
updated_data = await self.client.get_weather_data(
self.config_entry.data[CONF_LATITUDE],
self.config_entry.data[CONF_LONGITUDE],
self.supported_data_sets,
hourly_start=now,
hourly_end=now + timedelta(days=7),
)
homeassistant/components/weatherkit/coordinator.py:76
- This change introduces new request parameters (
hourly_start/hourly_end) and a new 7-day window, but the WeatherKit tests do not assert thatget_weather_datais called with the expected arguments/window. Adding a coordinator test that freezes time and verifies the API client call parameters would guard against regressions and accidental range changes.
updated_data = await self.client.get_weather_data(
self.config_entry.data[CONF_LATITUDE],
self.config_entry.data[CONF_LONGITUDE],
self.supported_data_sets,
hourly_start=now,
hourly_end=now + timedelta(days=7),
homeassistant/components/weatherkit/coordinator.py:71
nowis computed as a timezone-aware UTC datetime, but this coordinator still uses naivedatetime.now()forlast_updated_atand the stale-data threshold checks. Mixing aware/naive datetimes in the same method is easy to trip over later and can lead to subtle time math issues if more logic is added. Consider using a consistent UTC time source for all timestamps in this coordinator (for example, keep everything UTC-aware or everything UTC-naive) and reusing the samenowvalue for the update bookkeeping.
now = datetime.now(tz=UTC)
updated_data = await self.client.get_weather_data(
|
Out of curiosity, I checked the API docs:
So the default hourly window is actually 10 days; this PR is less than the default. Fair point, @joelhawksley However, my concern isn't the window size per se, it's that this data is fetched on every coordinator refresh. Pulling a week of hourly forecasts every 5 minutes means re-fetching ~168 data points whose values are essentially unchanged, with only the leading edge stale. That doesn't smell right to me and - once merged - all users of this integration are opted in with no way out. @tjhorner, what do you think? HA has a responsibility to be a good citizen with vendor APIs. The principled fix would be either a separate (slower) refresh cadence for the extended forecast (e.g. hourly data >24h is fetched every hour), or exposing this as a service/action that users can schedule themselves if they want it. |
Not true, because the library defaults to 1 day if not provided.
This is not necessarily true. The predictions will change based on current conditions.
I agree in principle, but in practice I don't think this would place much additional stress on a service from a company the size of Apple. The WeatherKit traffic from Home Assistant instances will be imperceptible compared to all Apple devices globally pinging it at regular intervals. The primary concern, in my opinion, is staying within the stated API limits. Since the limits are metered per request, the changes in this PR will not affect that. Keep in mind that end users are paying for access to this service as well; as long as we adhere to the WeatherKit ToS then I don't see an issue. |
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
zxdavb
left a comment
There was a problem hiding this comment.
Re tests: if the integration is now retrieving 7 days of hourly data instead of 24 hours - why hasn't there been a need to update a fixture or a snapshot?
|
@zxdavb from what I can tell, the test hardcodes the response data size so there was no need to change a fixture or snapshot. |
I think there is no value in taking that further. |
|
LGTM. |
Proposed change
This PR updates weatherkit to fetch hourly data for 7 days instead of 1.
While WeatherKit will return up to 10 days, I figured a week was fine. I want to be able to display hourly weather for the next couple of days, not just the next 24 hours.
Type of change
Additional information
N/A
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: