-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use a different cache key for the PR automation workflow #32588
Conversation
@@ -29,11 +29,13 @@ jobs: | |||
with: | |||
# npm cache files are stored in `~/.npm` on Linux/macOS | |||
path: ~/.npm | |||
key: ${{ runner.os }}-node-${{ matrix.node }}-npm-cache-${{ hashFiles('**/package-lock.json') }} | |||
key: ${{ runner.os }}-node-${{ matrix.node }}-npm-pr-automation-cache-${{ hashFiles('**/package-lock.json') }} |
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.
Do you think jobs that use npm ci
and npm install
should have different names as well?
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 think it should be OK to share caches between those workflows.
The cache is not the node_modules
folder, but rather the global level cache on the machine in ~/.npm
. The only difference between the two is that npm ci
it installs the exact packages listed in the lock file, and will return an error if a needed dependency is not found in that lock file. If the dependency is not present in the cache when using either command, it will just fetch it from NPM.
We also guard against discrepancies between the two by running npm install
in the static analysis workflow and checking for local changes.
Size Change: 0 B Total Size: 1.03 MB ℹ️ View Unchanged
|
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.
This doesn't seem to solve all issues but let's get it in and see.
* Use a different cache key for the PR automation workflow. * Update the `chalk` dependency to the latest version. (cherry picked from commit 05ed04a)
* Use a different cache key for the PR automation workflow. * Update the `chalk` dependency to the latest version. (cherry picked from commit 05ed04a)
(cherry picked from commit 05ed04a)
…33174) * Only calculate the compressed size when necessary. (#32161) This updates the compressed size workflow to run only when the files changes would have an impact on the package size. (cherry picked from commit 23d84b3) * Limit when workflows run on forks (#32114) (cherry picked from commit 435dde4) * Improvements to NPM package caching across workflows (#32458) * Use a consistent cache key for NPM packages across all workflows. Also, include the NodeJS version in the cache key in case different package versions are required. * Update the `actions/cache` action to the latest version. * Remove the strategy matrix from jobs with a single NodeJS version. For some reason, GitHub Actions will attach matrix values to job names in parenthesis. Because required checks are configured by job name, these need to stay consistent. (cherry picked from commit 7e54162) * Limit when release artifacts are built on forks: pt. 2 (#32494) * Limit when release artifacts are built on forks. Previously in #32114, adjustments were made to GHA workflow files to limit when workflows ran on forked repositories. But this missed the second job in the workflow that builds the release artifact. Because it was set to always run even though the previous job is required, it has continued to run on forks. * Ensure the artifact is built always, even when the preceding job does not run. (cherry picked from commit 8fbc4d6) * Use a different cache key for the PR automation workflow (#32588) * Use a different cache key for the PR automation workflow. * Update the `chalk` dependency to the latest version. (cherry picked from commit 05ed04a) * Fix flaky widgets-related E2E tests (#33066) * Remove empty keys from the compared object * Revert dev artifacts * Disable the gutenberg-test-marquee-widget plugin * Wrap marquee tests in their own describe statement * Lint * Update snapshots * Replace hello with howdy * Move plugin activation to beforeEach * Move deleteAllWidgets to beforeEach * Update tests * use data-testid rather than name attribute selectors * Remove any existing marquees before running the tests, use the "save" form button * Remove dev artifact Co-authored-by: Kai Hao <[email protected]> (cherry picked from commit 5cafe02) * Navigation: skip flakey tests (#33074) (cherry picked from commit 4d0959e) Co-authored-by: Adam Zielinski <[email protected]> Co-authored-by: Kerry Liu <[email protected]>
…33174) * Only calculate the compressed size when necessary. (#32161) This updates the compressed size workflow to run only when the files changes would have an impact on the package size. (cherry picked from commit 23d84b3) * Limit when workflows run on forks (#32114) (cherry picked from commit 435dde4) * Improvements to NPM package caching across workflows (#32458) * Use a consistent cache key for NPM packages across all workflows. Also, include the NodeJS version in the cache key in case different package versions are required. * Update the `actions/cache` action to the latest version. * Remove the strategy matrix from jobs with a single NodeJS version. For some reason, GitHub Actions will attach matrix values to job names in parenthesis. Because required checks are configured by job name, these need to stay consistent. (cherry picked from commit 7e54162) * Limit when release artifacts are built on forks: pt. 2 (#32494) * Limit when release artifacts are built on forks. Previously in #32114, adjustments were made to GHA workflow files to limit when workflows ran on forked repositories. But this missed the second job in the workflow that builds the release artifact. Because it was set to always run even though the previous job is required, it has continued to run on forks. * Ensure the artifact is built always, even when the preceding job does not run. (cherry picked from commit 8fbc4d6) * Use a different cache key for the PR automation workflow (#32588) * Use a different cache key for the PR automation workflow. * Update the `chalk` dependency to the latest version. (cherry picked from commit 05ed04a) * Fix flaky widgets-related E2E tests (#33066) * Remove empty keys from the compared object * Revert dev artifacts * Disable the gutenberg-test-marquee-widget plugin * Wrap marquee tests in their own describe statement * Lint * Update snapshots * Replace hello with howdy * Move plugin activation to beforeEach * Move deleteAllWidgets to beforeEach * Update tests * use data-testid rather than name attribute selectors * Remove any existing marquees before running the tests, use the "save" form button * Remove dev artifact Co-authored-by: Kai Hao <[email protected]> (cherry picked from commit 5cafe02) * Navigation: skip flakey tests (#33074) (cherry picked from commit 4d0959e) Co-authored-by: Adam Zielinski <[email protected]> Co-authored-by: Kerry Liu <[email protected]>
…33174) * Only calculate the compressed size when necessary. (#32161) This updates the compressed size workflow to run only when the files changes would have an impact on the package size. (cherry picked from commit 23d84b3) * Limit when workflows run on forks (#32114) (cherry picked from commit 435dde4) * Improvements to NPM package caching across workflows (#32458) * Use a consistent cache key for NPM packages across all workflows. Also, include the NodeJS version in the cache key in case different package versions are required. * Update the `actions/cache` action to the latest version. * Remove the strategy matrix from jobs with a single NodeJS version. For some reason, GitHub Actions will attach matrix values to job names in parenthesis. Because required checks are configured by job name, these need to stay consistent. (cherry picked from commit 7e54162) * Limit when release artifacts are built on forks: pt. 2 (#32494) * Limit when release artifacts are built on forks. Previously in #32114, adjustments were made to GHA workflow files to limit when workflows ran on forked repositories. But this missed the second job in the workflow that builds the release artifact. Because it was set to always run even though the previous job is required, it has continued to run on forks. * Ensure the artifact is built always, even when the preceding job does not run. (cherry picked from commit 8fbc4d6) * Use a different cache key for the PR automation workflow (#32588) * Use a different cache key for the PR automation workflow. * Update the `chalk` dependency to the latest version. (cherry picked from commit 05ed04a) * Fix flaky widgets-related E2E tests (#33066) * Remove empty keys from the compared object * Revert dev artifacts * Disable the gutenberg-test-marquee-widget plugin * Wrap marquee tests in their own describe statement * Lint * Update snapshots * Replace hello with howdy * Move plugin activation to beforeEach * Move deleteAllWidgets to beforeEach * Update tests * use data-testid rather than name attribute selectors * Remove any existing marquees before running the tests, use the "save" form button * Remove dev artifact Co-authored-by: Kai Hao <[email protected]> (cherry picked from commit 5cafe02) * Navigation: skip flakey tests (#33074) (cherry picked from commit 4d0959e) Co-authored-by: Adam Zielinski <[email protected]> Co-authored-by: Kerry Liu <[email protected]>
Description
In #32458, cache keys were made more consistent in order to increase the likelihood a cache hit would occur, which speeds up
npm install
andnpm ci
commands.However, this introduced a bug where the Pull Request Automation workflow would also share an NPM cache with all of the other workflows. This is problematic because this workflows runs
npm install
from within thepackages/project-management-automation
directory (which only installs dependencies needed for this package). When this workflow runs and attempts to write the cache first (which is likely to happen often, as this workflow usually completes before other workflows even get started), this results in the NPM cache for Node 14 being bad or incomplete.This can be seen here. The
package-lock.json
file changed in the connected commit (734fc41), causing a cache invalidation (a portion of the cache key is the hash of thepackage-lock.json
file), and the results of this workflow were cached.The Build Plugin ZIP workflow, which started enough time after the cache was created then used this small cache.
This only affects jobs running on
ubuntu-latest
with NodeJS 14. MacOS and NodeJS 12 jobs have different cache keys.I am also updating the
chalk
dependency in this PR in order to trigger a cache flush. The two releases since the version being used currently have only minor changes.Related
There have been an increased number of workflows failing randomly that display the following error:
npm ERR! cb() never called!
. Based on some quick Googling, it seems the recommended remedy for this isnpm cache clean --force
, which indicates it's being caused by a bad cache. The hope is that this change will also fix these random failures happening after the cache key is reset.Checklist:
*.native.js
files for terms that need renaming or removal).