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



##########
File path: pkg/webservice/handlers.go
##########
@@ -82,14 +82,23 @@ func getClusterInfo(w http.ResponseWriter, r *http.Request) 
{
 func getApplicationsInfo(w http.ResponseWriter, r *http.Request) {
        writeHeaders(w)
 
+       queueName := r.URL.Query().Get("queue")
        var appsDao []*dao.ApplicationDAOInfo
        lists := gClusterInfo.ListPartitions()
        for _, k := range lists {
                partition := gClusterInfo.GetPartition(k)
                appList := partition.GetApplications()
                for _, app := range appList {
                        appDao := getApplicationJSON(app)
-                       appsDao = append(appsDao, appDao)
+                       if len(queueName) > 0 {
+                               queueNameArr := strings.Split(app.QueueName, 
".")

Review comment:
       This should be done outside of the loop. It will not change inside the 
loop.

##########
File path: pkg/webservice/handlers.go
##########
@@ -82,14 +82,23 @@ func getClusterInfo(w http.ResponseWriter, r *http.Request) 
{
 func getApplicationsInfo(w http.ResponseWriter, r *http.Request) {
        writeHeaders(w)
 
+       queueName := r.URL.Query().Get("queue")
        var appsDao []*dao.ApplicationDAOInfo
        lists := gClusterInfo.ListPartitions()
        for _, k := range lists {
                partition := gClusterInfo.GetPartition(k)
                appList := partition.GetApplications()
                for _, app := range appList {
                        appDao := getApplicationJSON(app)
-                       appsDao = append(appsDao, appDao)
+                       if len(queueName) > 0 {
+                               queueNameArr := strings.Split(app.QueueName, 
".")
+                               appQueueName := 
queueNameArr[len(queueNameArr)-1]

Review comment:
       If the app queue name is just the leaf queue than we cannot do this.
   The query could be set to `root.test` and the apps runs in `root.sub.test` 
and your filter will return it as `test` is equal.
   
   We should have the full queue path in the app to correctly filter and check 
all parts of the query parameter.

##########
File path: pkg/webservice/handlers.go
##########
@@ -82,14 +82,23 @@ func getClusterInfo(w http.ResponseWriter, r *http.Request) 
{
 func getApplicationsInfo(w http.ResponseWriter, r *http.Request) {
        writeHeaders(w)
 
+       queueName := r.URL.Query().Get("queue")

Review comment:
       the parameter must be sanity checked to make sure it is a queue:
   * must be name <dot> name ...
   * each name must comply with QueueNameRegExp (see the configs code for that)
   * anything that does not comply give an error or be ignored

##########
File path: pkg/webservice/handlers.go
##########
@@ -82,14 +82,23 @@ func getClusterInfo(w http.ResponseWriter, r *http.Request) 
{
 func getApplicationsInfo(w http.ResponseWriter, r *http.Request) {
        writeHeaders(w)
 
+       queueName := r.URL.Query().Get("queue")
        var appsDao []*dao.ApplicationDAOInfo
        lists := gClusterInfo.ListPartitions()
        for _, k := range lists {
                partition := gClusterInfo.GetPartition(k)
                appList := partition.GetApplications()
                for _, app := range appList {
                        appDao := getApplicationJSON(app)
-                       appsDao = append(appsDao, appDao)
+                       if len(queueName) > 0 {

Review comment:
       Filtering should happen before we get the whole object




----------------------------------------------------------------
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:
[email protected]


Reply via email to