Skip to content

Cache Open-Meteo JSON reponse locally to reduce the number of API calls and add resilience#503

Merged
davidusb-geek merged 12 commits into
davidusb-geek:masterfrom
paulhomes:weather
Apr 18, 2025
Merged

Cache Open-Meteo JSON reponse locally to reduce the number of API calls and add resilience#503
davidusb-geek merged 12 commits into
davidusb-geek:masterfrom
paulhomes:weather

Conversation

@paulhomes
Copy link
Copy Markdown
Contributor

This change caches the default 3-day Open-Meteo JSON response locally with a configurable time limit (30 minutes by default). It is intended to resolve issue #492 where the post-subset data stored in the pickle weather_forecast_cache becomes stale quickly and, with regular scheduled 5 min MPCs, it results in frequent Open-Meteo API calls for weather data that does not need to be updated that frequently. I am currently running this in my test emhass docker instance where the Open-Meteo JSON response is cached using this change and I have turned off the pickle weather_forecast_cache.

It is only caching the JSON response from Open-Meteo so there are no changes to the existing pickle weather_forecast_cache code and it should not impact caching for any of the other weather methods.

By keeping the full 3 days in the JSON cache, and continuing to use it if there are network errors, it also means that emhass will be more resilient to any transient connectivity issues with Open-Meteo.

If you like this suggested change then I can try adding a configuration parameter for max_age for those that might want to change the default (via REST and/or config.json)

What are your preferences for a default? I picked 30 minutes to start with but it could be longer like an hour or maybe 3 or 4 hours.

@paulhomes paulhomes marked this pull request as ready for review April 9, 2025 23:12
@davidusb-geek
Copy link
Copy Markdown
Owner

Hi, there is a pending revision on this PR, can you please check it?

@paulhomes
Copy link
Copy Markdown
Contributor Author

Hi David, sorry I am not sure I understand what the pending revision is. I did ask a question about if you would like a configuration parameter for max_age (via REST and/or config.json). If so then I will change this PR back to draft and keep working on it.

@davidusb-geek
Copy link
Copy Markdown
Owner

Hi David, sorry I am not sure I understand what the pending revision is. I did ask a question about if you would like a configuration parameter for max_age (via REST and/or config.json). If so then I will change this PR back to draft and keep working on it.

If you scroll up this PR you can see that I left this message:

@paulhomes Could you please keep this commented code snippet?
I know it's ugly but it is used from time to time to save the needed file to mock a request to be used in unit tests

@paulhomes
Copy link
Copy Markdown
Contributor Author

Hi David, that's really strange as when I look at this PR, the first message I see from you is the one about the pending revision. I don't see any prior messages other than my initial opening comment and the commits (see screenshot below). Could this be a github bug?

I can see the commented code block I removed though. Sorry about that. I'll add it back in today.

image

@davidusb-geek
Copy link
Copy Markdown
Owner

Oh I see, that's very strange indeed you cannot see any review comment. Seems like a github bug

@sonarqubecloud
Copy link
Copy Markdown

@paulhomes
Copy link
Copy Markdown
Contributor Author

I also added a configuration option for the max age of the cached open-meteo json so it can be set differently to the default of 30 minutes if required.

Comment thread src/emhass/forecast.py
+ quote(str(self.time_zone), safe="")
)
response = get(url, headers=headers)
"""import bz2 # Uncomment to save a serialized data for tests
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@paulhomes Could you please keep this commented code snippet?
I know it's ugly but it is used from time to time to save the needed file to mock a request to be used in unit tests

@davidusb-geek davidusb-geek merged commit 6b679f4 into davidusb-geek:master Apr 18, 2025
14 checks passed
@paulhomes paulhomes deleted the weather branch April 18, 2025 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants