fix: add Cache-Control max-age to CDN files #76
Merged
Conversation
This file contains 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
anuraaga
approved these changes
Jan 11, 2022
| @@ -3,15 +3,15 @@ | |||
| version=$(npm pkg get version | sed 's/"//g') | |||
| bucket=$1 | |||
|
|
|||
| aws s3api put-object --bucket $bucket --key "content/$version/cwr.js" --body build/assets/cwr.js | |||
| aws s3api put-object --bucket $bucket --key "content/$version/LICENSE-THIRD-PARTY" --body LICENSE-THIRD-PARTY | |||
| aws s3api put-object --bucket $bucket --key "content/$version/cwr.js" --body build/assets/cwr.js --cache-control max-age=604800 | |||
There was a problem hiding this comment.
Does it make sense to set these even longer, like 1 year?
There was a problem hiding this comment.
I'm not sure. It's about risk -- what is the likelihood that a specific version will need to be modified?
In theory a version should never change once deployed, but my preference is still to reduce risk, at least for now, by limiting a potential impact to one week instead of one year.
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.
Partially resolves #73 .
This change adds a Cache-Control header to new web client versions deployed to CDN. Specific versions (e.g.,
1.0.4) have a max age of one week. Auto-update versions (e.g.,1.0.xand1.x) have a max age of two hours, so that impact is limited in case a change needs be be rolled back.Note that browsers typically cache files by default (using a heuristic to estimate TTL), so this change is non-critical.
We still need to add Cache-Control headers to the existing CDN files to fully resolve #73. This will be done by directly modifying the header info of the existing files.
Testing
I ran the deployment workflow in the gamma environment and verified the header
cache-control: max-age=604800is present in1.0.4/cwr.js, andcache-control: max-age=7200is present in1.0.x/cwr.jsand1.x/cwr.js. The Lighthouse recommendation no longer appeared for1.0.4/cwr.js.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.