[GitHub] [incubator-yunikorn-core] yangwwei commented on a change in pull request #268: YUNIKORN-645. Fixed status code issues, error handling issues etc

[GitBox <gi...@apache.org>]

yangwwei commented on a change in pull request #268:
URL: https://github.com/apache/incubator-yunikorn-core/pull/268#discussion_r620870903



##########
File path: pkg/webservice/webservice.go
##########
@@ -81,7 +82,7 @@ type YResponseWriter struct {
 
 func (rw *YResponseWriter) Write(bytes []byte) (int, error) {
 	rw.body = bytes
-	if rw.statusCode == -1 {
+	if rw.statusCode == -1 || (rw.statusCode >= http.StatusOK && rw.statusCode <= http.StatusIMUsed) {

Review comment:
        I still have concerns about checking the status code. This doesn't seem to be a reliable solution. I suggest reverting the wrapper change and continue to use the default writer, which is safer for me. If you look at https://golang.org/pkg/net/http/#Client.Get, the go client library will return an error with some internal logic and check over the HTTP status code. If the wrapper on the server-side wrongly operates on responses, we are in trouble. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org