nienbo / cache-buildkite-plugin

Tarball, Rsync & S3 Cache Kit for Buildkite. Supports Linux, macOS and Windows
https://buildkite.com/plugins
MIT License
67 stars 39 forks source link

feat: Adding 'save-cache' property for S3 cache #57

Closed ingcsmoreno closed 1 year ago

ingcsmoreno commented 1 year ago

What it does?

It makes the plugin saves the tar file on /tmp after downloading it for the first time, and re-uses that file on any further execution.

This is to complement the #36 implementation in further executions of builds on the same node, and when the same cache file is used by multiple pipelines.

How can I test it?

I've created a test to check it works.

Also, use this Plugin from this Fork on a pipeline, and enable the property like this:

...

  - plugins:
    - gencer/cache#v2.4.13:
        id: ruby # or ruby-3.0
        backend: s3
        key: "v1-cache-{{ id }}-{{ runner.os }}-{{ checksum 'Gemfile.lock' }}"
        restore-keys:
          - 'v1-cache-{{ id }}-{{ runner.os }}-'
          - 'v1-cache-{{ id }}-'
        s3:
          prefere-local: true
        paths:
          - 'bundle/vendor'
...

Issues

Extra Comments

This is not really to fully address #36 but to complement it.

gencer commented 1 year ago

Hey @ingcsmoreno!

Thanks for the PR. I see the point and I think this is a great feature, but I'm bit unsure about naming: prefere-local. Should it be prefer-local or something else? If you have any other suggestion or word about this, let me know, Otherwise I'll merge the PR today late-evening.


As a bonus, thanks for the version bump & release note addition too. A great favor for me. I usually await for few features until a bump but nowadays I just release even single features too.

ingcsmoreno commented 1 year ago

Hey @ingcsmoreno!

Thanks for the PR. I see the point and I think this is a great feature, but I'm bit unsure about naming: prefere-local. Should it be prefer-local or something else? If you have any other suggestion or word about this, let me know, Otherwise I'll merge the PR today late-evening.

As a bonus, thanks for the version bump & release note addition too. A great favor for me. I usually await for few features until a bump but nowadays I just release even single features too.

Thanks! I agree the property name could be more descriptive, but I don't want it to be too long either. Maybe something like add-local-cache, local-cache-enabled, prevent-download-if-local? I'm open to suggestions :D

gencer commented 1 year ago
...

  - plugins:
    - gencer/cache#v2.4.13:
        id: ruby # or ruby-3.0
        backend: s3
        key: "v1-cache-{{ id }}-{{ runner.os }}-{{ checksum 'Gemfile.lock' }}"
        restore-keys:
          - 'v1-cache-{{ id }}-{{ runner.os }}-'
          - 'v1-cache-{{ id }}-'
        s3:
          cache: true # keep the cache between builds/jobs on the same machine
        paths:
          - 'bundle/vendor'
...

I think only one word cache will work much better than 2-3 words with dashes... Or if we want more descriptive key:

keep-cache: true # keep the cache between builds/jobs on the same machine
# or
save-cache: true # save the cache on temp folder and keep between builds/jobs on the same machine

As soon as we put this information on README it doesn't matter how short or long the key is.

ingcsmoreno commented 1 year ago

@gencer save-cache sounds good, I've just updated the PR and files. Thanks!

gencer commented 1 year ago

@ingcsmoreno As you might notice, CI fails on this PR.

ingcsmoreno commented 1 year ago

@gencer looks like it's complaining about the s3 -> profile yaml syntax? https://github.com/nienbo/cache-buildkite-plugin/blob/07a1c2b323bf1d695b6d5c64febfe9402071c5bf/plugin.yml#L29

I executed the lint on master and it also fails with the same issue. Doesn't seem to be related with my changes. IMHO seems to be something related with the linter image code.

Could you give me a hand to figure this out?

ingcsmoreno commented 1 year ago

@gencer The CICD check is fixed!

gencer commented 1 year ago

@ingcsmoreno Released as https://github.com/nienbo/cache-buildkite-plugin/releases/tag/v2.4.13 🚀