Added delay before purging jsDelivr cache #23107
Conversation
ref https://linear.app/ghost/issue/PROD-1573 - last week, purging jsDelivr cache after publishing a new NPM version of admin-x-activitypub was unreliable: - we released a new version (0.6.68) of our JS package, admin-x-activitypub, last Thursday 24/04 around ~17:45 UTC. The cache was purged immediately after and Mike could see the latest version on my production site - but, on the next day, Mike opened my production site again and was served a previous version. Same browser / same location (no VPN) - our release process: 1. We bump the version of a package in a commit 2. Once the commit is merged to our main branch, our CI checks whether a new version has been created, and if yes, publishes a new version to npm (open-source code →) 3. Once a new version has been published, CI immediately clears jsDelivr cache - after reaching out to jsDelivr, the suggested fix was to add a 1-minute delay between publishing a new NPM version and purging jsDelivr cache - we're adding the 1-min delay in this comment and will monitor / open back the investigation if problem happens again
WalkthroughThe GitHub Actions workflow configuration (.github/workflows/ci.yml) was modified to add a conditional waiting step before purging the jsDelivr cache. Specifically, if the package version has changed and the package is Suggested labels
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (13)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
1333-1338: Consider using a dedicated wait action for clarity
The inlinesleep 60correctly implements the one-minute delay before purging the jsDelivr cache as recommended. To make the intent more explicit and maintainable, you could swap the shell block for a standard GitHub Action:- name: Wait 60 seconds uses: actions/wait@v1 with: time: 60This avoids inline scripting and clearly signals the purpose of the step.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: Signup-form tests
- GitHub Check: Regression tests (Node 20.11.1, sqlite3)
- GitHub Check: Ghost-CLI tests
- GitHub Check: Regression tests (Node 20.11.1, mysql8)
- GitHub Check: Comments-UI tests
- GitHub Check: Database tests (Node 20.11.1, sqlite3)
- GitHub Check: Admin-X Settings tests
- GitHub Check: Unit tests (Node 22.13.1)
- GitHub Check: Database tests (Node 22.13.1, mysql8)
- GitHub Check: Unit tests (Node 20.11.1)
- GitHub Check: Database tests (Node 20.11.1, mysql8)
- GitHub Check: Unit tests (Node 18.12.1)
- GitHub Check: Database tests (Node 18.12.1, mysql8)
- GitHub Check: Admin tests - Chrome
|
From reading the issue, maybe it's happened more than once? Sorry if I missed that! |
|
@ErisDS Two occurrences that we've noticed so far, but in general it's about being sure that a change we expect to be in production is really in production. Use case for ActivityPub: we ship changes and communicate them in the weekly newsletter; by the time the newsletter is sent, we'd like to be sure the change is in production for all users. Our process: ship a new version of the AP frontend, then purge jsDelivr cache to make sure all users are being served the latest version (in theory) What's happening sometimes (2 times so far): we think that we've shipped a new version to all users, but jsDelivr is still serving an old version; making our release pipeline unreliable/unpredictable. — This change should not add one minute delay to all CI runs: the delay is only added to commits on main that contains a version bump for either ActivityPub frontend, Portal, Sodo Search or Comments. All other CI runs should be unaffected by the change. Is that more acceptable? |
|
Yeah I realise it's only added to CI runs that affect the apps, but it still feels like the wrong fix to me. I apologise if I'm being dumb, but I don't understand why the jsDelivr cache needs purging at all - so I'm trying to see if there's a way to delete that requirement by fixing something else? |
|
FWIW short term, I think you can add Then we can take our time to figure out what the right solution is |
|
@ErisDS We load When a new version of Now by default, jsDelivr caches assets for up to 7 days. So unless we actively ask jsDelivr to purge their cache, they won't be serving the latest version instantly to all production users. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #23107 +/- ##
==========================================
- Coverage 71.86% 71.85% -0.01%
==========================================
Files 1432 1435 +3
Lines 99403 99433 +30
Branches 12211 12219 +8
==========================================
+ Hits 71435 71450 +15
- Misses 26984 26994 +10
- Partials 984 989 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@ErisDS I have restricted the wait step for the |
@ErisDS Oups missed that you're off this week! Based on your previous message, I will assume you're in board with the test as long as it's bounded to |
ref https://linear.app/ghost/issue/PROD-1573
last week, purging jsDelivr cache after publishing a new NPM version of admin-x-activitypub was unreliable:
release process:
after reaching out to jsDelivr, the suggested fix was to add a 1-minute delay between publishing a new NPM version and purging jsDelivr cache
we're adding the 1-min delay in this commit and will monitor / open back the investigation if problem happens again