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.

Sign up for GitHub

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

Jump to bottom

chore(ci): set a shorter cache on manifest #6660

Merged
merged 2 commits into from
May 23, 2024
Merged

chore(ci): set a shorter cache on manifest #6660

merged 2 commits into from
May 23, 2024

Conversation

binoy14
Copy link
Contributor

@binoy14 binoy14 commented May 14, 2024 •
edited
Loading

Description

Update the cache header on the manifest to be 10s and JS files to 1 year immutable

FIXES SDX-1343

What to review

Changes makes sense

Testing

N/A

Notes for release

N/A (internal)

@vercel Vercel
Copy link

vercel bot commented May 14, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
performance-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 22, 2024 8:31pm
test-next-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 22, 2024 8:31pm
test-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 22, 2024 8:31pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
studio-workshop ⬜️ Ignored (Inspect) Visit Preview May 22, 2024 8:31pm

@binoy14 Graphite App
Copy link
Contributor Author

binoy14 commented May 14, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @binoy14 and the rest of your teammates on Graphite Graphite

@github-actions GitHub Actions
Copy link
Contributor

No changes to documentation

binoy14
binoy14 commented May 14, 2024
View reviewed changes
scripts/uploadBundles.ts Outdated
@@ -160,6 +160,10 @@ async function updateManifest(newVersions: Map<string, string>) {
const options = {
destination: 'modules/v1/manifest-v1.json',
contentType: 'application/json',
metadata: {
// 10 mins cache
cacheControl: 'public, max-age=600',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rexxars should this be shorter than even 10 mins?

Copy link
Member

Choose a reason for hiding this comment

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

(I got assigned because of a force push to next 😇)
note that the only way to "guarantee" fresh content is to cache break when you load the manifest file. There will most likely be at least two caches involved, client and google cdn, and they will both use this one cache header. This will lead to max cache time being between 600 and (600+600).
So if you're able to add a static hash, that's unique per build of the manifest, when the user loads the file it would reduce the cache time to whenever the cache is broken on the page that loads it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What if the manifest is only used on the server? In that case won't it just be the (600) from the cdn?

Copy link
Member

Choose a reason for hiding this comment

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

Depends on how many proxies and caches are in the chain, but if it's just a http call to a gcp bucket it should only be up to 600s

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep that is what it is, I'll DM you to confirm. Thanks for jumping in

@binoy14 binoy14 marked this pull request as ready for review May 14, 2024 14:10
@binoy14 binoy14 requested a review from a team as a code owner May 14, 2024 14:10
@binoy14 binoy14 requested a review from bjoerge May 14, 2024 14:10
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 14, 2024 •
edited
Loading

Component Testing Report Updated May 22, 2024 8:38 PM (UTC)

File Status Duration Passed Skipped Failed
comments/CommentInput.spec.tsx ✅ Passed (Inspect) 35s 15 0 0
formBuilder/ArrayInput.spec.tsx ✅ Passed (Inspect) 6s 3 0 0
formBuilder/inputs/PortableText/Annotations.spec.tsx ✅ Passed (Inspect) 25s 6 0 0
formBuilder/inputs/PortableText/copyPaste/CopyPaste.spec.tsx ✅ Passed (Inspect) 31s 11 7 0
formBuilder/inputs/PortableText/Decorators.spec.tsx ✅ Passed (Inspect) 14s 6 0 0
formBuilder/inputs/PortableText/DisableFocusAndUnset.spec.tsx ✅ Passed (Inspect) 8s 3 0 0
formBuilder/inputs/PortableText/FocusTracking.spec.tsx ✅ Passed (Inspect) 36s 15 0 0
formBuilder/inputs/PortableText/Input.spec.tsx ✅ Passed (Inspect) 1m 16s 21 0 0
formBuilder/inputs/PortableText/ObjectBlock.spec.tsx ✅ Passed (Inspect) 1m 3s 18 0 0
formBuilder/inputs/PortableText/PresenceCursors.spec.tsx ✅ Passed (Inspect) 7s 3 9 0
formBuilder/inputs/PortableText/RangeDecoration.spec.tsx ✅ Passed (Inspect) 20s 9 0 0
formBuilder/inputs/PortableText/Styles.spec.tsx ✅ Passed (Inspect) 14s 6 0 0
formBuilder/inputs/PortableText/Toolbar.spec.tsx ✅ Passed (Inspect) 30s 12 0 0

@binoy14 binoy14 requested a review from rexxars May 14, 2024 15:01
@binoy14 binoy14 requested review from a team and sjelfull and removed request for a team May 14, 2024 15:52
@bjoerge
Copy link
Member

bjoerge commented May 15, 2024

Needs a rebase?

@bjoerge bjoerge removed request for bjoerge and a team May 15, 2024 08:52
@binoy14 binoy14 force-pushed the 05-14-chore_ci_set_a_small_cache_on_manifest branch from 53a2995 to 77f6990 Compare May 15, 2024 13:44
@ricokahler ricokahler requested review from a team as code owners May 15, 2024 21:56
@sgulseth sgulseth removed the request for review from a team May 16, 2024 12:55
@binoy14 binoy14 force-pushed the 05-14-chore_ci_set_a_small_cache_on_manifest branch from 77f6990 to 9565e3b Compare May 16, 2024 14:42
@binoy14 binoy14 removed request for sjelfull and a team May 21, 2024 13:20
@binoy14 binoy14 marked this pull request as draft May 22, 2024 18:48
@binoy14 binoy14 force-pushed the 05-14-chore_ci_set_a_small_cache_on_manifest branch from 9565e3b to 92df7ef Compare May 22, 2024 20:29
@binoy14 binoy14 marked this pull request as ready for review May 22, 2024 20:30
rexxars
rexxars approved these changes May 23, 2024
View reviewed changes
Copy link
Member

@rexxars rexxars left a comment

Choose a reason for hiding this comment

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

👍

@binoy14 binoy14 added this pull request to the merge queue May 23, 2024
Merged via the queue into next with commit 069dd31 May 23, 2024
40 checks passed
@binoy14 binoy14 deleted the 05-14-chore_ci_set_a_small_cache_on_manifest branch May 23, 2024 18:24
@leocabrallce leocabrallce mentioned this pull request Jun 20, 2024
Open
This was referenced Jun 24, 2024
Open
Open
@leocabrallce leocabrallce mentioned this pull request Jun 27, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sgulseth sgulseth sgulseth left review comments

@rexxars rexxars rexxars approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@binoy14 @bjoerge @rexxars @sgulseth