Remove Much of the HTTP API Surface Area #84
Conversation
arschles
added
the
kind/feature
|
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://github.com/kubernetes/kubernetes/wiki/CLA-FAQ to sign the CLA. Once you've signed, please reply here (e.g. "I signed it!") and we'll verify. Thanks.
If you have questions or suggestions related to this bot's behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
|
2 similar comments
|
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://github.com/kubernetes/kubernetes/wiki/CLA-FAQ to sign the CLA. Once you've signed, please reply here (e.g. "I signed it!") and we'll verify. Thanks.
If you have questions or suggestions related to this bot's behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
|
|
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://github.com/kubernetes/kubernetes/wiki/CLA-FAQ to sign the CLA. Once you've signed, please reply here (e.g. "I signed it!") and we'll verify. Thanks.
If you have questions or suggestions related to this bot's behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
|
k8s-ci-robot
added
the
cncf-cla: no
|
|
||
| port := strconv.Itoa(s.port) | ||
| log.Println("Server started on port " + port) | ||
| err := http.ListenAndServe(":"+port, nil) | ||
| log.Println(err.Error()) | ||
| } | ||
|
|
||
| func healthZHandler(w http.ResponseWriter, r *http.Request) { | ||
| w.WriteHeader(http.StatusOK) |
There was a problem hiding this comment.
Consider adding a TODO here to do some actual health checking?
There was a problem hiding this comment.
@vaikas-google I'll add an issue for it instead. cool with you?
There was a problem hiding this comment.
I strongly prefer issues over TODOs in the code personally.
In accordance with kubernetes-retired#54, this patch attempts to only remove the API surface area. It won’t refactor any code. Fixes kubernetes-retired#54
|
|
| k8sHandler: k8sHandler, | ||
| controller: c, | ||
| port: port, | ||
| k8sHandler: k8sHandler, | ||
| }, nil | ||
| } | ||
|
|
||
| // Start starts the server and begins listening on a TCP port. | ||
| func (s *Server) Start() { | ||
| router := mux.NewRouter() |
There was a problem hiding this comment.
@arschles can I suggest adding router.StrictSlash(true) after this line?
There was a problem hiding this comment.
yep, good idea. added
|
LGTM (again). |
|
|
||
| port := strconv.Itoa(s.port) | ||
| log.Println("Server started on port " + port) | ||
| err := http.ListenAndServe(":"+port, nil) | ||
| log.Println(err.Error()) | ||
| } | ||
|
|
||
| func healthZHandler(w http.ResponseWriter, r *http.Request) { | ||
| w.WriteHeader(http.StatusOK) |
There was a problem hiding this comment.
I strongly prefer issues over TODOs in the code personally.
|
LGTM |
In accordance with #54, this patch attempts to only remove the API surface area. It won’t refactor any code.
Fixes #54