[GitHub] [incubator-yunikorn-core] wilfred-s commented on a change in pull request #186: [YUNIKORN-283] Support query params to filter the apps using queue name/namespace for v1/apps endpoint

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

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



##########
File path: pkg/webservice/handlers_test.go
##########
@@ -175,3 +177,83 @@ func TestContainerHistory(t *testing.T) {
 	assert.Equal(t, contHist[0].TotalContainers, "2", "metric 1 should be 1 apps and was not")
 	assert.Equal(t, contHist[4].TotalContainers, "300", "metric 5 should be 300 apps and was not")
 }
+
+func TestQueryParamInAppsHandler(t *testing.T) {
+	const configDefault = `
+partitions:
+  - name: default
+    queues:
+      - name: root
+        queues:
+        - name: default
+`
+	rmID := "rm-123"
+	policyGroup := "default-policy-group"
+	clusterInfo := cache.NewClusterInfo()
+
+	configs.MockSchedulerConfigByData([]byte(configDefault))
+	if _, err := cache.SetClusterInfoFromConfigFile(clusterInfo, rmID, policyGroup); err != nil {
+		t.Errorf("Error when load clusterInfo from config %v", err)

Review comment:
       nit: `t.Fatalf()` gives you this in one go.

##########
File path: pkg/webservice/handlers.go
##########
@@ -98,6 +107,20 @@ func getApplicationsInfo(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
+func validateQueue(queueName string) error {
+	if queueName != "" {
+		queueNameArr := strings.Split(queueName, ".")
+		for _, name := range queueNameArr {
+			if !configs.QueueNameRegExp.MatchString(name) {
+				return fmt.Errorf("problem in queue query parameter parsing as queue param " +
+					"#{queueName} contains invalid queue name #{name}. Queue name must only have " +

Review comment:
       Not sure what happened here but this does not achieve what you want: use `%s` for the strings see [fmt.Errorf example](https://golang.org/pkg/fmt/#Errorf)




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