[GitHub] [incubator-yunikorn-core] wilfred-s commented on a change in pull request #194: [YUNIKORN-273] expose config via rest

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

wilfred-s commented on a change in pull request #194:
URL: https://github.com/apache/incubator-yunikorn-core/pull/194#discussion_r464939200



##########
File path: pkg/webservice/handlers.go
##########
@@ -291,3 +293,33 @@ func getContainerHistory(w http.ResponseWriter, r *http.Request) {
 	}
 
 }
+
+func getClusterConfig(w http.ResponseWriter, r *http.Request) {
+	writeHeaders(w)
+	jsonOutput := false

Review comment:
       changed, default is yaml

##########
File path: pkg/webservice/handlers.go
##########
@@ -291,3 +293,33 @@ func getContainerHistory(w http.ResponseWriter, r *http.Request) {
 	}
 
 }
+
+func getClusterConfig(w http.ResponseWriter, r *http.Request) {
+	writeHeaders(w)
+	jsonOutput := false
+	query := r.URL.Query()
+	// check if we have a request for json output
+	if len(query) != 0 {
+		if _, ok := query["json"]; ok {
+			jsonOutput = true
+		}
+	}
+
+	var marshalledConf []byte
+	var err error
+	conf := configs.ConfigContext.Get(gClusterInfo.GetPolicyGroup())
+	if jsonOutput {
+		marshalledConf, err = json.Marshal(&conf)
+	} else {
+		marshalledConf, err = yaml.Marshal(&conf)

Review comment:
       REST does not specify, require or limit anything we return it can be any text form as well as binary.
   
   We should provide yaml as that is the format the original file is in. We should not expect that the client **must** implement a json to yaml conversion tool before it can use the file. It might also lead to data issues.
   




----------------------------------------------------------------
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