Update caching strategy to include node version #1543
Conversation
.circleci/config.yml
Outdated
| - v2-dependencies-{{ checksum "package-lock.json" }} | ||
| # fallback to using the latest cache if no exact match is found | ||
| - v2-dependencies- | ||
| - node-14-{{ .Branch }}-{{ checksum "package-lock.json" }} |
There was a problem hiding this comment.
do we need to include the branch here? it also might still be helpful to have a version number if we decide to change this again
There was a problem hiding this comment.
I can add the version back.
Including the branch with the fallback is simply an optimization recommended by Circle CI where if a cache is made for a particular branch but then the package-lock.json changes, the cache from the same branch will be restored on subsequent builds. When the npm install is ran in the next step, it only needs to install the dependencies that have changed. It's not necessary for fixing this issue so I can remove it though
There was a problem hiding this comment.
I thought that adding the branch would de-optimize (un-optimize? english?) cases across branches though? like if I change the deps in dev/bump-jest then merge it into develop, develop wouldn't use the cache from dev/bump-jest
There was a problem hiding this comment.
In your example, if we change deps in dev/bump-jest and merge into develop, develop would use the latest cache that starts with 'v3-node-14-develop', and then run npm install to get the new dependencies. So you're right, it would wouldn't use the dev/bump-jest cache. Including branch tells the caching strategy to prefer prior caches from the same branch. If we don't include branch in the cache key and we change a dependency, it will restore the latest cache that starts with 'v3-node-14', which could be any branch, so it will likely grab the cache from dev/bump-jest, but it could be a cache from a different branch. It makes a small difference either way, so I will remove it
There was a problem hiding this comment.
Sorry I know I'm really drilling into this, but do we want this fallback behavior? If the package-lock and node version are different shouldn't we do a fresh npm install? Like it's possible the buggy behavior nidhi ran into is because of the fallback to v1-dependencies
There was a problem hiding this comment.
No, you're good. The fallback behavior is recommended because it means that you will frequently restore the latest node modules cache, and when the npm install runs in the next step, it only needs to install the dependencies that have changed. It's safe to do on NPM 5+ and it helps speed the builds up by causing more cache hits. By adding the node version to the key, it won't fallback to caches built with different node versions
There was a problem hiding this comment.
got it! so basically just because we find a cache key doesn't mean we skip npm install
There was a problem hiding this comment.
BUT if the node version is different now we won't find a cache key so no node-sass conflicts (or any other libraries that build things like C++ bindings)
There was a problem hiding this comment.
Yep, if you view Circle CI, you will see it always runs the npm install regardless of whether or not the cache was hit. If there'a a cache hit it saves us about 25 seconds on the build
cea2aj
changed the title
Update caching strategy to include node version I looked into best practices and I wasn't able to find anyone that ran into problems with different node versions of the cache, however a similar problem frequently occurs when different architectures are used to build dependencies, and the solution is to include the architecture of the system in the cache key. So this approach is similar except we are adding the node version rather than the architecture to the cache key. We can include the node version dynamically, however we would need to migrate over to Circle CI dynamic configuration in order to supply the node version as a parameter to the circle CI config. J=SLAP-1547 TEST=manual Inspect the cache key in Circle CI and confirm that it is correct
Update caching strategy to include node version I looked into best practices and I wasn't able to find anyone that ran into problems with different node versions of the cache, however a similar problem frequently occurs when different architectures are used to build dependencies, and the solution is to include the architecture of the system in the cache key. So this approach is similar except we are adding the node version rather than the architecture to the cache key. We can include the node version dynamically, however we would need to migrate over to Circle CI dynamic configuration in order to supply the node version as a parameter to the circle CI config. J=SLAP-1547 TEST=manual Inspect the cache key in Circle CI and confirm that it is correct
Update caching strategy to include node version I looked into best practices and I wasn't able to find anyone that ran into problems with different node versions of the cache, however a similar problem frequently occurs when different architectures are used to build dependencies, and the solution is to include the architecture of the system in the cache key. So this approach is similar except we are adding the node version rather than the architecture to the cache key. We can include the node version dynamically, however we would need to migrate over to Circle CI dynamic configuration in order to supply the node version as a parameter to the circle CI config. J=SLAP-1547 TEST=manual Inspect the cache key in Circle CI and confirm that it is correct
Update caching strategy to include node version I looked into best practices and I wasn't able to find anyone that ran into problems with different node versions of the cache, however a similar problem frequently occurs when different architectures are used to build dependencies, and the solution is to include the architecture of the system in the cache key. So this approach is similar except we are adding the node version rather than the architecture to the cache key. We can include the node version dynamically, however we would need to migrate over to Circle CI dynamic configuration in order to supply the node version as a parameter to the circle CI config. J=SLAP-1547 TEST=manual Inspect the cache key in Circle CI and confirm that it is correct
Update caching strategy to include node version I looked into best practices and I wasn't able to find anyone that ran into problems with different node versions of the cache, however a similar problem frequently occurs when different architectures are used to build dependencies, and the solution is to include the architecture of the system in the cache key. So this approach is similar except we are adding the node version rather than the architecture to the cache key. We can include the node version dynamically, however we would need to migrate over to Circle CI dynamic configuration in order to supply the node version as a parameter to the circle CI config. J=SLAP-1547 TEST=manual Inspect the cache key in Circle CI and confirm that it is correct
Update caching strategy to include node version I looked into best practices and I wasn't able to find anyone that ran into problems with different node versions of the cache, however a similar problem frequently occurs when different architectures are used to build dependencies, and the solution is to include the architecture of the system in the cache key. So this approach is similar except we are adding the node version rather than the architecture to the cache key. We can include the node version dynamically, however we would need to migrate over to Circle CI dynamic configuration in order to supply the node version as a parameter to the circle CI config. J=SLAP-1547 TEST=manual Inspect the cache key in Circle CI and confirm that it is correct
Update caching strategy to include node version
I looked into best practices and I wasn't able to find anyone that ran into problems with different node versions of the cache, however a similar problem frequently occurs when different architectures are used to build dependencies, and the solution is to include the architecture of the system in the cache key. So this approach is similar except we are adding the node version rather than the architecture to the cache key.
We can include the node version dynamically, however we would need to migrate over to Circle CI dynamic configuration in order to supply the node version as a parameter to the circle CI config. If the team would prefer that, I can update this PR to do that. Having dynamic config could be useful in other ways in the future.
J=SLAP-1547
TEST=manual
Inspect the cache key in Circle CI and confirm that it is correct