Skip to content
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

Fix JSON file download failure fallback #14497

Merged
merged 2 commits into from Feb 4, 2023

Conversation

Rylan12
Copy link
Member

@Rylan12 Rylan12 commented Feb 4, 2023

Follow-up to #14491

We want to actually use the old JSON file if we're falling back. Currently, we "fall back" by returning nothing which causes CaskLoader to assume that the API is not a viable option and skipping to loading from taps if they're available. Instead, we want to warn and then use the old version.

I think this will help with the weird cask loading errors we've been seeing. My best theory at the moment is that they happen when the API download fails and then it accidentally falls back to the taps.


I think this will close #14483

@Rylan12 Rylan12 added the critical Critical change which should be shipped as soon as possible. label Feb 4, 2023
@BrewTestBot
Copy link
Member

Review period skipped due to critical label.

@Rylan12 Rylan12 mentioned this pull request Feb 4, 2023
3 tasks
@Rylan12 Rylan12 added the install from api Relates to API installs label Feb 4, 2023
@apainintheneck
Copy link
Contributor

This makes sense to me. I've actually been wondering if we could fall back to the cached JSON more often like when the command doesn't require the latest and greatest formulae/cask list. I assume that any commands that wouldn't work correctly with an out-of-date list can't be used with HOMEBREW_INSTALL_FROM_API anyway (like live check for example).

@MikeMcQuaid
Copy link
Member

I think this needs tweaked a little as if the second JSON parse fails it won't be caught properly. I'll push t the PR.

@MikeMcQuaid
Copy link
Member

I've actually been wondering if we could fall back to the cached JSON more often like when the command doesn't require the latest and greatest formulae/cask list.

Yeh, I think we could avoid checking e.g. more than once a minute for the newest things?

@MikeMcQuaid MikeMcQuaid merged commit 7b3756a into Homebrew:master Feb 4, 2023
@Rylan12 Rylan12 deleted the fix-api-fallback branch February 4, 2023 13:23
@github-actions github-actions bot added the outdated PR was locked due to age label Mar 7, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
critical Critical change which should be shipped as soon as possible. install from api Relates to API installs outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API-based update downgraded cask
4 participants