Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2k
Update font URLs and add additional error handling#7418
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
.circleci/config.yml Outdated
| install-geckodriver:false | ||
| install-chrome:true | ||
| chrome-version:"127.0.6533.119" | ||
| chrome-version:"136.0.7103.92" |
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.
why this change?
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.
CI wasfailing with a message about "Chrome failed to start" and this resolved the issue; in any case, it seems good to keep the Chrome version somewhat up to date.
.circleci/download_google_fonts.py Outdated
| outfile=dir_out+name | ||
| print("Getting: ",url) | ||
| ifos.path.exists(outfile): | ||
| print(" => Already exists: ",outfile) |
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.
rather than overwrite (I don't know how often font files themselves update)?
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.
I did this instead of removing the download step from the CI; in other words, the download script will still run but it will not actually attempt to download the fonts because they are already in the repo.
I could alternatively modify the script to overwrite, and remove the step that runs the script from the CI. I liked this approach though because it's easier to revert if we change our minds about keeping the fonts in the repo.
I don't think we're worried about the font files being updated all that often... if we were I wouldn't want to put them in the repo.
b560368 intomasterUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
We have been getting "529 Too Many Requests" errors when downloading fonts in the CI.
It seems these were due to multiple redirects happening for each font URL.
I've added the font files directly into the repo to sidestep all of this in the future (I don't love adding 10MB of size to the repo, and open to removing them in the future if it causes issues, but I also don't want to be spending any more time fixing the font downloads every few weeks)
I've updated the URLs in the script to use the "final" URL to avoid redirects, and this seems to resolve the issue -- download works successfully now
I've also set
allow_redirects=Falsein the request and added logging to print the new URL so the script can be updated, so we can catch this earlier next timeI've updated the script to skip downloading the fonts if the file is already present, so while the download script will still run in the CI, it won't actually execute the downloads since the files are in the repo now
I've also updated the
.circleci/env_image.shscript so that it will fail if any step fails, which was not the case before, which made this issue harder to debug