-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Set artifact caching on by default #2621
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
Set artifact caching on by default #2621
Conversation
bb4b2ea
to
b77fc97
Compare
Codecov Report
|
integration/debug_test.go
Outdated
deployments: []string{"jib"}, | ||
pods: []string{"nodejs", "npm", "python3"}, | ||
}, | ||
{ | ||
description: "kustomize", | ||
args: []string{"--profile", "kustomize"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a reason why we turn off caching for these two tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
never mind. just saw your commit message "Add --cache-artifacts=false for integrationt tests where we check builds"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but these are debug test? do we still need to disable caching?
4749467
to
67074fe
Compare
6e28bff
to
b02554b
Compare
This integration test for caching makes sure a build event is sent even if all artifacts are cached. This is necessry for the IDEs, which use the initial build event to determine a new dev loop has started.
cc @nkubala could you PTAL at the event additions and integration test? |
eventing and integration test look fine. in the future it might be cool to add some information in the build event itself about whether or not it was built from cache, something like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
merge when CI is green!
Fixes #2511