pdctl: verify pd address #528
Conversation
pdctl/command/global.go
Outdated
| if err != nil { | ||
| return err | ||
| } | ||
| if reps.StatusCode != http.StatusOK { |
There was a problem hiding this comment.
must read all response body and close
pdctl/ctl.go
Outdated
| command.InitPDClient(rootCmd) | ||
| err := command.InitPDClient(rootCmd) | ||
| if err != nil { | ||
| return err.Error(), err |
There was a problem hiding this comment.
why return err.Error() here?
There was a problem hiding this comment.
this string will print.
There was a problem hiding this comment.
why not check error and print the error directly?
| @@ -87,7 +87,9 @@ func loop() { | |||
| } | |||
| continue | |||
| } | |||
|
|
|||
| if line == "exit" { | |||
There was a problem hiding this comment.
if InitPDClient meet error, it will return error directly without execute the command. so I move exit to here.
| command.InitPDClient(rootCmd) | ||
| err := command.InitPDClient(rootCmd) | ||
| if err != nil { | ||
| fmt.Println(err) |
There was a problem hiding this comment.
should exit here directly?
|
PTAL @disksing |
pdctl/command/global.go
Outdated
| } | ||
| defer reps.Body.Close() | ||
| if reps.StatusCode != http.StatusOK { | ||
| ioutil.ReadAll(reps.Body) |
There was a problem hiding this comment.
You don't have to read it.
There was a problem hiding this comment.
If not read, the connection FD will not close even you call body close later. I remember this is a pitfall before, but don't know go improves it or not.
Btw, We should call ioutil.ReadAll(reps.Body) before the status check.
|
LGTM |
close #507
PTAL @siddontang @overvenus @huachaohuang