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

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

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



##########
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:
       I think its good to see the default the response as yaml.
   As I see, its not good to compare headers like r.Header.Get. If possible, could we some how have a cleaner interfaces to find/get some header if present. Some time, comparison can have issues in unicode, caps etc etc. It's better we have a util package/class to handle some checks in a standard and use some bool/enum to handle the further processing.

##########
File path: pkg/webservice/routes.go
##########
@@ -24,74 +24,82 @@ import (
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 )
 
-type Route struct {
+type route struct {
 	Name        string
 	Method      string
 	Pattern     string
 	HandlerFunc http.HandlerFunc
 }
 
-type Routes []Route
+type routes []route
 
-var routes = Routes{
+var webRoutes = routes{
 	// endpoints to retrieve general scheduler info
-	Route{
+	route{
 		"Scheduler",
 		"GET",
 		"/ws/v1/queues",
 		getQueueInfo,
 	},
-	Route{
+	route{
 		"Cluster",
 		"GET",
 		"/ws/v1/clusters",
 		getClusterInfo,
 	},
-	Route{
+	route{
 		"Scheduler",
 		"GET",
 		"/ws/v1/apps",
 		getApplicationsInfo,
 	},
-	Route{
+	route{
 		"Scheduler",
 		"GET",
 		"/ws/v1/nodes",
 		getNodesInfo,
 	},
 
 	// endpoint to retrieve goroutines info
-	Route{
+	route{
 		"Scheduler",
 		"GET",
 		"/ws/v1/stack",
 		getStackInfo,
 	},
 
 	// endpoint to retrieve server metrics
-	Route{
+	route{
 		"Scheduler",
 		"GET",
 		"/ws/v1/metrics",
 		promhttp.Handler().ServeHTTP,
 	},
 
+	// endpoint to retrieve the current conf
+	route{
+		"Scheduler",
+		"GET",
+		"/ws/v1/config",
+		getClusterConfig,
+	},
+
 	// endpoint to validate conf
-	Route{
+	route{

Review comment:
       I think this is not a correct way to handle a POST for validate. Recheck with REST mode.
   
   Also, its better we clearly move all PUT, POST, DELETE etc to another cleaner interface or under some group so that we can protect all WRITE ops in a more secure way later from simple GET calls.




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