Added NodeJS version to GHA cache keys #23277
Conversation
WalkthroughThe CI workflow configuration is updated to change how dependency cache keys are generated and used. The cache key is now split into a base key and a full key that includes the Node.js version. All jobs that restore dependency caches are updated to use the new key format, ensuring the Node.js version is part of the key. The automatic Yarn cache in the Node setup step is disabled. The 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (7)
🔇 Additional comments (1)
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 (
|
cmraible
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
148-150: Clarify variable naming and usage in setting full cache key
The step writes to an env var namedcachekey, then later outputs it asdependency_cache_key. For clarity and consistency, consider renamingcachekeyhere todependency_cache_keyand reference$NODE_VERSIONin your shell context:- run: echo "cachekey=${cachekey_base}-node-${{ env.NODE_VERSION }}" >> "$GITHUB_ENV" + run: echo "dependency_cache_key=${cachekey_base}-node-${NODE_VERSION}" >> "$GITHUB_ENV"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Regression tests (Node 20.11.1, mysql8)
- GitHub Check: Signup-form tests
- GitHub Check: Admin tests - Chrome
- GitHub Check: Ghost-CLI tests
- GitHub Check: Comments-UI tests
- GitHub Check: Admin-X Settings tests
🔇 Additional comments (4)
.github/workflows/ci.yml (4)
217-219: Welcome addition ofdependency_cache_key_baseoutput
Addingdependency_cache_key_baseto the job outputs enables downstream matrix jobs to build version-specific keys. This aligns well with the PR objective.
476-479: Use base cache key in unit-tests matrix
Good update—usingdependency_cache_key_basewith-node-${{ matrix.node }}dynamically restores the correct cache per Node version.
552-555: Use base cache key in database-tests matrix
Consistent with unit-tests, this correctly restores caches per Node version.
680-684: Use base cache key in regression-tests matrix
Great—you’re now restoring dependency caches with the proper Node version suffix.
.github/workflows/ci.yml
Outdated
| echo "dep-cache-${{ env.hash }}-${{ github.sha }}-node-${{ env.NODE_VERSION }}" >> "$GITHUB_ENV" | ||
| echo "dep-cache-${{ env.hash }}-node-${{ env.NODE_VERSION }}" >> "$GITHUB_ENV" | ||
| echo "dep-cache-node-${{ env.NODE_VERSION }}" >> "$GITHUB_ENV" |
There was a problem hiding this comment.
Restore keys should be derived from computed variables
Right now you rebuild fallback keys with env.hash and hard-coded patterns, which won’t match your generated cachekey_base or dependency_cache_key. Instead, reuse those values:
- echo "dep-cache-${{ env.hash }}-${{ github.sha }}-node-${{ env.NODE_VERSION }}" >> "$GITHUB_ENV"
- echo "dep-cache-${{ env.hash }}-node-${{ env.NODE_VERSION }}" >> "$GITHUB_ENV"
- echo "dep-cache-node-${{ env.NODE_VERSION }}" >> "$GITHUB_ENV"
+ echo "${{ env.dependency_cache_key }}" >> "$GITHUB_ENV"
+ echo "${{ env.cachekey_base }}-node-${{ env.NODE_VERSION }}" >> "$GITHUB_ENV"
+ echo "${{ env.cachekey_base }}" >> "$GITHUB_ENV"
+ echo "dep-cache-node-${{ env.NODE_VERSION }}" >> "$GITHUB_ENV"This ensures your restore-keys actually line up with the keys you generate.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo "dep-cache-${{ env.hash }}-${{ github.sha }}-node-${{ env.NODE_VERSION }}" >> "$GITHUB_ENV" | |
| echo "dep-cache-${{ env.hash }}-node-${{ env.NODE_VERSION }}" >> "$GITHUB_ENV" | |
| echo "dep-cache-node-${{ env.NODE_VERSION }}" >> "$GITHUB_ENV" | |
| echo "${{ env.dependency_cache_key }}" >> "$GITHUB_ENV" | |
| echo "${{ env.cachekey_base }}-node-${{ env.NODE_VERSION }}" >> "$GITHUB_ENV" | |
| echo "${{ env.cachekey_base }}" >> "$GITHUB_ENV" | |
| echo "dep-cache-node-${{ env.NODE_VERSION }}" >> "$GITHUB_ENV" |
|
I'd be surprised if this helped - Node.js provides the Node-API for binary dependencies and this is stable across different Node versions. So binaries built for one Node-API can be run on any Node.js version that supports that Node-API version Def strange that we're seeing an increase in this recently Not a blocking comment but just my 2c |
|
Ah okay that makes sense. I still wonder if the NX cache accounts for all that when caching build outputs 🤔 |
no refs
TL;DR: we run some of our test suites against multiple versions of Node, but they are currently sharing the same caches. Dependencies that are built for one version may not work for the other version, causing flaky behavior in CI. This commit adds the NodeJS version to the cache keys that we use in CI, so that each version will maintain its own cache. This should hopefully reduce flakiness in our CI workflows.
Here's a recent error from comments-ui unit tests that failed this morning:
It's followed by a huge unintelligible (to me) native stack trace, but I'm told that "This is almost certainly caused by loading precompiled code (e.g. cached .node modules or bytecode caches) that were compiled with a different version of Node.js or V8."
This seems to track, as clearing the caches is our go-to for fixing these flaky issues we've been seeing..