-
Notifications
You must be signed in to change notification settings - Fork 106
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
When checking read/write access, if there is no access, surface the e… #1133
Conversation
…rror that occurred This can be helpful in debugging failed builds where access is expected Signed-off-by: Natalie Arellano <narellano@vmware.com>
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.
LGTM
Signed-off-by: Natalie Arellano <narellano@vmware.com>
return "", err | ||
} | ||
if ok { | ||
if ok, _ := checkReadAccess(image, keychain); ok { |
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.
Ideally we'd log the error at this point, but that requires a logger, which is a bigger change
Signed-off-by: Natalie Arellano <narellano@vmware.com>
if !img.CheckReadAccess() { | ||
canRead, err := img.CheckReadAccess() | ||
if !canRead { | ||
cmd.DefaultLogger.Debugf("Error checking read access: %s", err) |
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.
We should log that without using Debug as some users using Tekton or kpack could not be aware about how to enable log level !!
if !img.CheckReadWriteAccess() { | ||
canReadWrite, err := img.CheckReadWriteAccess() | ||
if !canReadWrite { | ||
cmd.DefaultLogger.Debugf("Error checking read/write access: %s", err) |
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.
We should log that without using Debug as some users using Tekton or kpack could not be aware about how to enable log level !!
…rror that occurred
This can be helpful in debugging failed builds where access is expected