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

Add CacheCommand to DockerCommand interface #336

Merged
merged 2 commits into from
Sep 7, 2018

Conversation

priyawadhwa
Copy link
Collaborator

CacheCommand returns true if the command should be cached. Currently,
it's only true for RUN but can be added to ADD/COPY later on (these are
different since the contents of files for ADD/COPY need to be included
in the cache key generation).

I also changed CreatedBy to String so that we can log each command
before cache extraction or regular execution takes place.

CacheCommand returns true if the command should be cached. Currently,
it's only true for RUN but can be added to ADD/COPY later on (these are
different since the contents of files for ADD/COPY need to be included
in the cache key generation).

I also changed CreatedBy to String so that we can log each command
before cache extraction or regular execution takes place.
Copy link
Contributor

@tejal29 tejal29 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. However, when is CacheCommand used in flow?
Are you going to add those changes in another PR?

Can you describe what the change is going to be?

@priyawadhwa
Copy link
Collaborator Author

For sure -- once some other open PRs are merged (#320) I'll start to piece all of these together and implement caching for kaniko. I'll use CacheCommand to determine if a command should be cached or not (part 1 will only cache RUN commands, later on we could add ADD/COPY).

@priyawadhwa priyawadhwa merged commit 637f14e into GoogleContainerTools:master Sep 7, 2018
@priyawadhwa priyawadhwa deleted the string branch September 7, 2018 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants