fix(github): don't cache failed GitHub API responses for the full 2-h…#4256
Open
chalfontchubby wants to merge 2 commits into
Open
fix(github): don't cache failed GitHub API responses for the full 2-h…#4256chalfontchubby wants to merge 2 commits into
chalfontchubby wants to merge 2 commits into
Conversation
…our window download_predbat_releases_url never checked the HTTP status before treating the response body as valid data - GitHub's API returns a JSON body even on errors (e.g. rate limiting), so a 403 was parsed fine and cached as if it were real release data for 2 hours. That meant a transient rate limit kept showing "Unable to download Predbat version information" for up to 2 hours after the limit had actually cleared. Now checks response.ok before parsing/caching, and on any failure (network error, non-2xx, bad JSON) falls back to the last successfully cached data instead of caching the failure - so the next 5-minute plan cycle retries rather than waiting out the full cache window. Also adds a request timeout, since this is an external call with no bound on how long it can hang.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes GitHub release fetching so transient GitHub API failures (e.g., rate limiting) aren’t cached as “valid” release data for the full 2-hour cache window, and adds coverage for non-2xx responses.
Changes:
- Add
response.okchecking + request timeout when fetching GitHub releases; on failures, return the last known cached data instead of caching the failure. - Avoid updating the cache stamp/data on non-2xx or JSON failures so the next 5-minute cycle retries promptly.
- Extend
test_githubcoverage for non-2xx responses with and without prior cached data.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/predbat/github.py | Adds non-2xx guarding, timeout, and “fallback to last good cache” behavior for GitHub releases fetching. |
| apps/predbat/tests/test_github.py | Adds tests ensuring non-2xx responses don’t populate/refresh cache and correctly fall back to stale cached data. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Author
|
(Hi Trefor - don't think we ever overlapped at IMG - I left in 2012 - Rik Allen. Used to run the video simulation team) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Symptom: After clearing problems with GitHub api calls for my account, present continues to complain about being unable to download GitHub version information, leading to som confusion as to the continuing problem. download_predbat_releases_url fetches the GitHub version and caches it for a 2 hour window to reduce api calls to a minimum.
However it never checked the HTTP status before treating the response body as valid data - GitHub's API returns a JSON body even on errors (e.g. rate limiting), so a 403 was parsed fine and cached as if it were real release data for 2 hours. That meant a transient rate limit kept showing "Unable to download Predbat version information" for up to 2 hours after the limit had actually cleared.
Now checks response.ok before parsing/caching, and on any failure (network error, non-2xx, bad JSON) falls back to the last successfully cached data instead of caching the failure - so the next 5-minute plan cycle retries rather than waiting out the full cache window. Also adds a request timeout, since this is an external call with no bound on how long it can hang.