Cleanup healthcheck code after V2 removal #13655
Conversation
serathius
force-pushed
the
health
branch
3 times, most recently
from
893813b to
81847a5
Compare
Codecov Report
@@ Coverage Diff @@
## main #13655 +/- ##
==========================================
- Coverage 72.73% 72.58% -0.16%
==========================================
Files 467 469 +2
Lines 38287 38262 -25
==========================================
- Hits 27847 27771 -76
- Misses 8642 8675 +33
- Partials 1798 1816 +18
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| if !first { | ||
| fmt.Fprintf(w, ",\n") | ||
| } | ||
| first = false |
There was a problem hiding this comment.
Can you move the first = false into the if branch?
There was a problem hiding this comment.
This file is extracted from server/etcdserver/api/etcdhttp/base.go just to keep one file per endpoint. Would prefer not to mix file structure refactors with logic ones. There is no unit tests for this code, so I try to avoid logical changes.
| @@ -414,7 +414,7 @@ func startProxy(cfg *config) error { | |||
| go func() { | |||
| lg.Info("v2 proxy started listening on client requests", zap.String("host", host)) | |||
| mux := http.NewServeMux() | |||
| etcdhttp.HandlePrometheus(mux) // v2 proxy just uses the same port | |||
| etcdhttp.HandleMetrics(mux) // v2 proxy just uses the same port | |||
There was a problem hiding this comment.
Is the comment "v2 proxy just uses the same port" still valid?
There was a problem hiding this comment.
Yes? there is no semantic change in this rename.
serathius
force-pushed
the
health
branch
2 times, most recently
from
6d207de to
d1fc112
Compare
|
You need to format the source code using |
serathius
force-pushed
the
health
branch
2 times, most recently
from
ed13c11 to
18c60e3
Compare
|
@serathius can you rebase this PR? I think we need to get this one merged firstly, so that I can rework my PR 13772 on top of this one. |
|
Done. |
|
Although there may be still some refactors to be done, this PR should be good to go, because it's at least an improvement on current implementation. This PR may have some impact on other PRs, can we get this one merged? @ptabor @spzala @serathius Could you rebase this PR, so as to make sure it doesn't break any pipelines? Previously we had an experience which was breaking the pipeline. |
Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.