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. |
smarterclayton
force-pushed
the
improve_config_error
branch
from
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.
smarterclayton
force-pushed
the
improve_config_error
branch
from
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.
Any reason for single case switch over if?
There was a problem hiding this comment.
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)switch {—
Reply to this email directly or view it on GitHub.
There was a problem hiding this comment.
Fine as is, if you expect cases to be added later.
|
LGTM |
j3ffml
added
the
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