Skip to content
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

Please consider using several partial keys in restore_cache #63

Closed
julienw opened this issue Nov 26, 2020 · 6 comments
Closed

Please consider using several partial keys in restore_cache #63

julienw opened this issue Nov 26, 2020 · 6 comments
Assignees

Comments

@julienw
Copy link

julienw commented Nov 26, 2020

Describe Request:

It is very common, and actually recommended in the CircleCI documentation, that we specify several keys in restore_cache, so that the cache can be reused and primed even if the underlying lock files change.

For example, Here is a real-life configuration from my project:

Saving is:

      - save_cache:
          paths:
            - node_modules
          key: v2-dependencies-{{ checksum "package.json" }}-{{ checksum "yarn.lock" }}

Restoring is:

      - restore_cache:
          keys:
            - v2-dependencies-{{ checksum "package.json" }}-{{ checksum "yarn.lock" }}
            - v2-dependencies-{{ checksum "package.json" }}-
            - v2-dependencies-

But this isn't what the orb is doing:

- when: # Restore cache for NPM
condition: << parameters.with-cache >>
steps:
- restore_cache:
keys:
- node-deps-<<parameters.cache-version>>-<<#parameters.include-branch-in-cache-key>>{{ .Branch }}-<</parameters.include-branch-in-cache-key>>{{ checksum "<<parameters.app-dir>>/package-lock.json" }}

- when: # Restore cache for YARN
condition: << parameters.with-cache >>
steps:
- restore_cache:
keys:
- node-deps-<<parameters.cache-version>>-<<#parameters.include-branch-in-cache-key>>{{ .Branch }}-<</parameters.include-branch-in-cache-key>>{{ checksum "<<parameters.app-dir>>/yarn.lock" }}

Is there a drawback to always doing this? Or should we make this a configuration option?

I'd be happy looking at this if there is consensus about it.

Thanks!

@patatepartie
Copy link

I would just add the {{ arch }} template, inside the key, as it's recommended in some parts of the docs and would help people with builds on different architectures, at no cost for those who don't.

@KyleTryon
Copy link
Contributor

Yes, unfortunately, because caching is achieved via CircleCI config and is not "programable", it would be difficult to support multiple cache keys via an orb. The addition of {{ arch }} however should assist some (I can't think of any downsides).

In semi relation, I have also opened this feature request for CircleCI to allow more programmatic access to cache and artifacts. Please feel free to vote here: https://ideas.circleci.com/api-feature-requests/p/programmatic-cache-and-artifact-access

@julienw
Copy link
Author

julienw commented Jan 11, 2021

I think it's quite easy to support the partial keys, what's more difficult is to make it configurable :-) But not sure there's a downside in having it all the time.

PiDelport added a commit to registreerocks/registree-core that referenced this issue Jan 25, 2021
PiDelport added a commit to registreerocks/registree-core that referenced this issue Jan 25, 2021
@Kurt-von-Laven
Copy link

Kurt-von-Laven commented May 22, 2021

Yeah, sorry, could you clarify that point, @KyleTryon? I don't see why this feature request would imply any special programmatic access to caches. The proposal as I understand it is to apply partial cache restoration as recommended by CircleCI. Having partial cache restoration all the time would break NPM users without a lock file as well as NPM users on version 4-. That being said, I don't personally feel there is a need for this feature being configurable at least in its first iteration since the command can inspect the presence of a lock file and npm --version and branch accordingly.

@KyleTryon
Copy link
Contributor

KyleTryon commented May 26, 2021

It looks like we could enable this as non-configurable.

@KyleTryon KyleTryon self-assigned this May 26, 2021
@KyleTryon
Copy link
Contributor

Have a solution prepared: #83

PiDelport added a commit to registreerocks/registree-core that referenced this issue Nov 10, 2021
Also drop link to
<CircleCI-Public/node-orb#63>: the issue has
been resolved, but there still isn't a partial restore key without the
branch name when include-branch-in-cache-key is enabled.
PiDelport added a commit to registreerocks/registree-core that referenced this issue Nov 10, 2021
* ci(circleci): fix Node versions to 16 (LTS), rather than 17

This avoids the following Yarn / Node 17 regression:

[Bug?]: node17 compat: │ Error [ERR_STREAM_PREMATURE_CLOSE]: Premature
close #3597

<yarnpkg/berry#3597>

(This has already been fixed in newer Yarn, but we want to stick to the
LTS anyway.)

* ci(circleci): update circleci/node orb, and use new yarn-berry support

Also drop link to
<CircleCI-Public/node-orb#63>: the issue has
been resolved, but there still isn't a partial restore key without the
branch name when include-branch-in-cache-key is enabled.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants