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
apiclient: don't log at error level when dialing #6830
apiclient: don't log at error level when dialing #6830
Conversation
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.
small tweaks
return conn, nil | ||
} | ||
if isX509Error(err) { | ||
if certErr := isX509Error(err); !a.HasNext() || certErr { |
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.
isCertErr would be a clearer variable name to not make you think that this was an error object, but is a boolean.
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.
done
} | ||
if !a.HasNext() { | ||
logger.Errorf("error dialing %q: %v", cfg.Location, err) | ||
logger.Debugf("error dialing %q (certificate error %v): %v", certErr, 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.
what do you think about:
detail := ""
if isCertErr {
detail = " (bad certificate)"
}
logger.Debugf("error dialing %q%s: %v", cfg.Location, detail, err)
PR juju#6620 changed the client logging so that it logs an error when it can't connect to an API address. It is common for these errors to occur when connecting even though the actual connect succeeds, so this PR changes the logging to debug level - it's part of the implementation but not something that users should always see.
5e4ac80
to
6f69b58
Compare
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
…-dial-2.1 apiclient: don't log at error level when dialing PR #6620 changed the client logging so that it logs an error when it can't connect to an API address. It is common for these errors to occur when connecting even though the actual connect succeeds, so this PR changes the logging to debug level - it's part of the implementation but not something that users should always see. This is a backport of #6830 to Juju 2.1, as the errors are confusing people and 2.2 won't be out for a while.
PR #6620 changed the client logging so that it logs an
error when it can't connect to an API address.
It is common for these errors to occur when connecting
even though the actual connect succeeds, so this
PR changes the logging to debug level - it's part
of the implementation but not something that users
should always see.