-
Notifications
You must be signed in to change notification settings - Fork 40.1k
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
Return a typed error for config validation, and make errors simple #6338
Return a typed error for config validation, and make errors simple #6338
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA) at https://cla.developers.google.com/. If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check the information on your CLA or see this help article on setting the email on your git commits. Once you've done that, please reply here to let us know. If you signed the CLA as a corporation, please let us know the company's name. |
3628176
to
b2be985
Compare
cc @jlowdermilk |
…specified Allows clients to distinguish between a server that returns us an error we recognize, and errors that are generic HTTP (due to an intervening proxy)
Will allow clients to determine when the configuration is bad.
Omit glog prefix when v < 2, show multiline errors for configuration problems, add new generic messages for server errors that hide some complexity that is not relevant for users.
97df1ab
to
323a44e
Compare
switch t := err.(type) { | ||
case *url.Error: | ||
glog.V(4).Infof("Connection error: %s %s: %v", t.Op, t.URL, t.Err) | ||
switch { |
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.
Any reason for single case switch over if?
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.
I had the original two cases and removed them. I can reduce to if, although I expect cases to be added here for other conditions eventually.
On Apr 3, 2015, at 12:50 PM, Jeff Lowdermilk notifications@github.com wrote:
In pkg/kubectl/cmd/util/helpers.go:
}
if client.IsUnexpectedStatusError(err) {
glog.FatalDepth(1, fmt.Sprintf("Unexpected status received from server: %s", err.Error()))
switch t := err.(type) {
case *url.Error:
glog.V(4).Infof("Connection error: %s %s: %v", t.Op, t.URL, t.Err)
Any reason for single case switch over if?switch {
—
Reply to this email directly or view it on GitHub.
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.
Fine as is, if you expect cases to be added later.
LGTM |
Kicking shippable, will merge on green. |
Return a typed error for config validation, and make errors simple
Will allow clients to determine when the configuration is bad.
On v < 2, we write to os.Stderr and exit 1 instead of calling glog.Fatalf
Follow on to #5016 and #3894