-
Notifications
You must be signed in to change notification settings - Fork 714
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
httpapi: support /status to show PD version #664
Conversation
Please sign the CLA @choleraehyq . |
@@ -0,0 +1,34 @@ | |||
package api |
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.
Add license header at the top of the file.
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.
server/api/status.go
Outdated
} | ||
|
||
func (h *statusHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { | ||
w.Header().Set("Content-Type", "application/json") |
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.
seem the default type is json, no need to set this, please ensure it.
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.
Yes. I removed it.
LGTM |
Please add test |
Signed-off-by: Cholerae Hu <choleraehyq@gmail.com>
Signed-off-by: Cholerae Hu <choleraehyq@gmail.com>
Signed-off-by: Cholerae Hu <choleraehyq@gmail.com>
@siddontang Test added. |
server/api/status_test.go
Outdated
resp, err := s.hc.Get(addr) | ||
c.Assert(err, IsNil) | ||
buf, err := ioutil.ReadAll(resp.Body) | ||
c.Assert(err, IsNil) |
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.
miss resp.Body.Close
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.
server/api/status_test.go
Outdated
|
||
for _, num := range numbers { | ||
cfgs, _, clean := mustNewCluster(c, num) | ||
defer clean() |
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.
don't call defer in for loop.
Signed-off-by: Cholerae Hu <choleraehyq@gmail.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
PTAL @disksing |
@disksing Can this be merged? |
See #551
PTAL @siddontang @nolouch
Signed-off-by: Cholerae Hu choleraehyq@gmail.com