Cache Node.js artifacts from maven-frontend-plugin on CI
#33815
Conversation
| - uses: actions/cache@v4 | ||
| name: Cache Node.js related artifacts | ||
| with: | ||
| path: | | ||
| ~/.m2/repository/com/github/eirslett/node | ||
| ~/.m2/repository/com/github/eirslett/pnpm | ||
| key: ${{ runner.os }}-frontend-plugin-artifacts |
There was a problem hiding this comment.
The cache key is static. Therefore it will be cached once, and the cache is never refreshed for a different node or NPM version, which is then downloaded all the time. Consider adding a combined cache key for npm & node.
This doesn't respect the setting create-cache-if-it-doesnt-exist. This could lead to a situation where there is no frontend being built, and then an empty cache is being created. Please create the cache only when this is set, and provide a download-only version when it is not set.
There was a problem hiding this comment.
The cache key is static. Therefore it will be cached once, and the cache is never refreshed for a different node or NPM version, which is then downloaded all the time. Consider adding a combined cache key for npm & node.
Done. The key for the cache now includes both the Node.js version and the PNPM version.
This doesn't respect the setting
create-cache-if-it-doesnt-exist. This could lead to a situation where there is no frontend being built, and then an empty cache is being created.
Like we discussed offline, the directory paths are very specific and would only create an artifact if the directories exist and contain files. Hence this should not be a problem for this specific cache action.
jonkoops
force-pushed
the
cache-nodejs-artifacts
branch
3 times, most recently
from
f930886
to
f72578e
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
![keycloak-github-bot[bot]](https://avatars.githubusercontent.com/u/4921466?s=60&v=4){kind=small}
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
jonkoops
force-pushed
the
cache-nodejs-artifacts
branch
2 times, most recently
from
8ea2480
to
0b0270b
Compare
Closes keycloak#31835 Signed-off-by: Jon Koops <[email protected]>
jonkoops
force-pushed
the
cache-nodejs-artifacts
branch
from
e58db6f
to
cf707b5
Compare
|
One problem that remains is that it is not possible to exclude the existing artifacts that are cached from |
Unreported flaky test detectedIf the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR. org.keycloak.testsuite.broker.KcOidcBrokerTest#testPostBrokerLoginFlowWithOTP_bruteForceEnabledKeycloak CI - Java Distribution IT (windows-latest - temurin - 17) |
…33815) Closes keycloak#31835 Signed-off-by: Jon Koops <[email protected]>
Closes #31835