You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@yunikorn.apache.org by GitBox <gi...@apache.org> on 2022/03/12 15:05:23 UTC

[GitHub] [incubator-yunikorn-web] akhilpb001 opened a new pull request #70: [YUNIKORN-955] Move the web UI calls to the new REST API endpoints

akhilpb001 opened a new pull request #70:
URL: https://github.com/apache/incubator-yunikorn-web/pull/70


   ### What is this PR for?
   A few sentences describing the overall goals of the pull request's commits.
   First time? Check out the contributing guide - http://yunikorn.apache.org/community/how_to_contribute   
   
   
   ### What type of PR is it?
   * [ ] - Bug Fix
   * [ ] - Improvement
   * [ ] - Feature
   * [ ] - Documentation
   * [ ] - Hot Fix
   * [ ] - Refactoring
   
   ### Todos
   * [ ] - Task
   
   ### What is the Jira issue?
   * Open an issue on Jira https://issues.apache.org/jira/browse/YUNIKORN/
   * Put link here, and add [YUNIKORN-*Jira number*] in PR title, eg. `[YUNIKORN-2] Gang scheduling interface parameters`
   
   ### How should this be tested?
   
   ### Screenshots (if appropriate)
   
   ### Questions:
   * [ ] - The licenses files need update.
   * [ ] - There is breaking changes for older versions.
   * [ ] - It needs documentation.
   


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

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

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



[GitHub] [incubator-yunikorn-web] craigcondit closed pull request #70: [YUNIKORN-955] Move the web UI calls to the new REST API endpoints

Posted by GitBox <gi...@apache.org>.
craigcondit closed pull request #70:
URL: https://github.com/apache/incubator-yunikorn-web/pull/70


   


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

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

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



[GitHub] [incubator-yunikorn-web] akhilpb001 commented on a change in pull request #70: [YUNIKORN-955] Move the web UI calls to the new REST API endpoints

Posted by GitBox <gi...@apache.org>.
akhilpb001 commented on a change in pull request #70:
URL: https://github.com/apache/incubator-yunikorn-web/pull/70#discussion_r828796143



##########
File path: src/app/services/scheduler/scheduler.service.ts
##########
@@ -237,52 +242,61 @@ export class SchedulerService {
   }
 
   private generateQueuesTree(data: any, currentQueue: QueueInfo) {
-    if (data && data.queues && data.queues.length > 0) {
+    if (data && data.children && data.children.length > 0) {
       const chilrenQs = [];
-      data.queues.forEach(queueData => {
+
+      data.children.forEach(queueData => {
         const childQueue = new QueueInfo();
-        childQueue.queueName = '' + queueData.queuename;
-        childQueue.state = queueData.status || 'RUNNING';
+
+        childQueue.queueName = queueData.queuename as string;
+        childQueue.status = queueData.status || 'n/a';

Review comment:
       Fixed




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

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

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



[GitHub] [incubator-yunikorn-web] akhilpb001 commented on a change in pull request #70: [YUNIKORN-955] Move the web UI calls to the new REST API endpoints

Posted by GitBox <gi...@apache.org>.
akhilpb001 commented on a change in pull request #70:
URL: https://github.com/apache/incubator-yunikorn-web/pull/70#discussion_r828796924



##########
File path: src/app/components/queue-rack/queue-rack.component.ts
##########
@@ -61,21 +61,16 @@ export class QueueRackComponent implements OnInit {
     this.queueSelected.emit(queue);
   }
 
+  // FIXME: Implement using absolute usage value in new /{partition}/queues REST API.
+  // Currently absolute usage value is not available in /{partition}/queues REST API.

Review comment:
       https://issues.apache.org/jira/browse/YUNIKORN-1128




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

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

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



[GitHub] [incubator-yunikorn-web] akhilpb001 commented on a change in pull request #70: [YUNIKORN-955] Move the web UI calls to the new REST API endpoints

Posted by GitBox <gi...@apache.org>.
akhilpb001 commented on a change in pull request #70:
URL: https://github.com/apache/incubator-yunikorn-web/pull/70#discussion_r828797443



##########
File path: src/app/services/scheduler/scheduler.service.ts
##########
@@ -167,68 +174,66 @@ export class SchedulerService {
     );
   }
 
-  public fetchNodeList(): Observable<NodeInfo[]> {
-    const nodesUrl = `${this.envConfig.getSchedulerWebAddress()}/ws/v1/nodes`;
+  fetchNodeList(partitionName: string): Observable<NodeInfo[]> {
+    const nodesUrl = `${this.envConfig.getSchedulerWebAddress()}/ws/v1/partition/${partitionName}/nodes`;
 
     return this.httpClient.get(nodesUrl).pipe(
       map((data: any) => {
         const result = [];
 
         if (data && data.length > 0) {
-          for (const info of data) {
-            const nodesInfoData = info.nodesInfo || [];
-
-            nodesInfoData.forEach(node => {
-              const nodeInfo = new NodeInfo(
-                node['nodeID'],
-                node['hostName'],
-                node['rackName'],
-                info['partitionName'],
-                this.formatCapacity(this.splitCapacity(node['capacity'], NOT_AVAILABLE)),
-                this.formatCapacity(this.splitCapacity(node['allocated'], NOT_AVAILABLE)),
-                this.formatCapacity(this.splitCapacity(node['occupied'], NOT_AVAILABLE)),
-                this.formatCapacity(this.splitCapacity(node['available'], NOT_AVAILABLE)),
-                []
-              );
-
-              const allocations = node['allocations'];
-              if (allocations && allocations.length > 0) {
-                const appAllocations = [];
-
-                allocations.forEach(alloc => {
-                  if (
-                    alloc.allocationTags &&
-                    alloc.allocationTags['kubernetes.io/meta/namespace'] &&
-                    alloc.allocationTags['kubernetes.io/meta/podName']
-                  ) {
-                    alloc[
-                      'displayName'
-                    ] = `${alloc.allocationTags['kubernetes.io/meta/namespace']}/${alloc.allocationTags['kubernetes.io/meta/podName']}`;
-                  } else {
-                    alloc['displayName'] = '<nil>';
-                  }
-                  appAllocations.push(
-                    new AllocationInfo(
-                      alloc['displayName'],
-                      alloc['allocationKey'],
-                      alloc['allocationTags'],
-                      alloc['uuid'],
-                      this.formatCapacity(this.splitCapacity(alloc['resource'], NOT_AVAILABLE)),
-                      alloc['priority'],
-                      alloc['queueName'],
-                      alloc['nodeId'],
-                      alloc['applicationId'],
-                      alloc['partition']
-                    )
-                  );
-                });
-
-                nodeInfo.setAllocations(appAllocations);
-              }
-
-              result.push(nodeInfo);
-            });
-          }
+          data.forEach(node => {
+            const nodeInfo = new NodeInfo(
+              node['nodeID'],
+              node['hostName'],
+              node['rackName'],
+              node['partition'] || NOT_AVAILABLE,

Review comment:
       @manirajv06 please add `partition` into the new the nodes response if it is not added or missing.




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

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

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



[GitHub] [incubator-yunikorn-web] craigcondit closed pull request #70: [YUNIKORN-955] Move the web UI calls to the new REST API endpoints

Posted by GitBox <gi...@apache.org>.
craigcondit closed pull request #70:
URL: https://github.com/apache/incubator-yunikorn-web/pull/70


   


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

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

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



[GitHub] [incubator-yunikorn-web] manirajv06 commented on a change in pull request #70: [YUNIKORN-955] Move the web UI calls to the new REST API endpoints

Posted by GitBox <gi...@apache.org>.
manirajv06 commented on a change in pull request #70:
URL: https://github.com/apache/incubator-yunikorn-web/pull/70#discussion_r826060878



##########
File path: src/app/services/scheduler/scheduler.service.ts
##########
@@ -237,52 +242,61 @@ export class SchedulerService {
   }
 
   private generateQueuesTree(data: any, currentQueue: QueueInfo) {
-    if (data && data.queues && data.queues.length > 0) {
+    if (data && data.children && data.children.length > 0) {
       const chilrenQs = [];
-      data.queues.forEach(queueData => {
+
+      data.children.forEach(queueData => {
         const childQueue = new QueueInfo();
-        childQueue.queueName = '' + queueData.queuename;
-        childQueue.state = queueData.status || 'RUNNING';
+
+        childQueue.queueName = queueData.queuename as string;
+        childQueue.status = queueData.status || 'n/a';

Review comment:
       Can you use constant "NOT_AVAILABLE" here as well?

##########
File path: src/app/services/scheduler/scheduler.service.ts
##########
@@ -167,68 +174,66 @@ export class SchedulerService {
     );
   }
 
-  public fetchNodeList(): Observable<NodeInfo[]> {
-    const nodesUrl = `${this.envConfig.getSchedulerWebAddress()}/ws/v1/nodes`;
+  fetchNodeList(partitionName: string): Observable<NodeInfo[]> {
+    const nodesUrl = `${this.envConfig.getSchedulerWebAddress()}/ws/v1/partition/${partitionName}/nodes`;
 
     return this.httpClient.get(nodesUrl).pipe(
       map((data: any) => {
         const result = [];
 
         if (data && data.length > 0) {
-          for (const info of data) {
-            const nodesInfoData = info.nodesInfo || [];
-
-            nodesInfoData.forEach(node => {
-              const nodeInfo = new NodeInfo(
-                node['nodeID'],
-                node['hostName'],
-                node['rackName'],
-                info['partitionName'],
-                this.formatCapacity(this.splitCapacity(node['capacity'], NOT_AVAILABLE)),
-                this.formatCapacity(this.splitCapacity(node['allocated'], NOT_AVAILABLE)),
-                this.formatCapacity(this.splitCapacity(node['occupied'], NOT_AVAILABLE)),
-                this.formatCapacity(this.splitCapacity(node['available'], NOT_AVAILABLE)),
-                []
-              );
-
-              const allocations = node['allocations'];
-              if (allocations && allocations.length > 0) {
-                const appAllocations = [];
-
-                allocations.forEach(alloc => {
-                  if (
-                    alloc.allocationTags &&
-                    alloc.allocationTags['kubernetes.io/meta/namespace'] &&
-                    alloc.allocationTags['kubernetes.io/meta/podName']
-                  ) {
-                    alloc[
-                      'displayName'
-                    ] = `${alloc.allocationTags['kubernetes.io/meta/namespace']}/${alloc.allocationTags['kubernetes.io/meta/podName']}`;
-                  } else {
-                    alloc['displayName'] = '<nil>';
-                  }
-                  appAllocations.push(
-                    new AllocationInfo(
-                      alloc['displayName'],
-                      alloc['allocationKey'],
-                      alloc['allocationTags'],
-                      alloc['uuid'],
-                      this.formatCapacity(this.splitCapacity(alloc['resource'], NOT_AVAILABLE)),
-                      alloc['priority'],
-                      alloc['queueName'],
-                      alloc['nodeId'],
-                      alloc['applicationId'],
-                      alloc['partition']
-                    )
-                  );
-                });
-
-                nodeInfo.setAllocations(appAllocations);
-              }
-
-              result.push(nodeInfo);
-            });
-          }
+          data.forEach(node => {
+            const nodeInfo = new NodeInfo(
+              node['nodeID'],
+              node['hostName'],
+              node['rackName'],
+              node['partition'] || NOT_AVAILABLE,

Review comment:
       Is node['partition'] available in response? Can you check?

##########
File path: src/app/components/queue-rack/queue-rack.component.ts
##########
@@ -61,21 +61,16 @@ export class QueueRackComponent implements OnInit {
     this.queueSelected.emit(queue);
   }
 
+  // FIXME: Implement using absolute usage value in new /{partition}/queues REST API.
+  // Currently absolute usage value is not available in /{partition}/queues REST API.

Review comment:
       Please raise a jira for this




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

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

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



[GitHub] [incubator-yunikorn-web] akhilpb001 commented on a change in pull request #70: [YUNIKORN-955] Move the web UI calls to the new REST API endpoints

Posted by GitBox <gi...@apache.org>.
akhilpb001 commented on a change in pull request #70:
URL: https://github.com/apache/incubator-yunikorn-web/pull/70#discussion_r828794601



##########
File path: src/app/services/scheduler/scheduler.service.ts
##########
@@ -167,68 +174,66 @@ export class SchedulerService {
     );
   }
 
-  public fetchNodeList(): Observable<NodeInfo[]> {
-    const nodesUrl = `${this.envConfig.getSchedulerWebAddress()}/ws/v1/nodes`;
+  fetchNodeList(partitionName: string): Observable<NodeInfo[]> {
+    const nodesUrl = `${this.envConfig.getSchedulerWebAddress()}/ws/v1/partition/${partitionName}/nodes`;
 
     return this.httpClient.get(nodesUrl).pipe(
       map((data: any) => {
         const result = [];
 
         if (data && data.length > 0) {
-          for (const info of data) {
-            const nodesInfoData = info.nodesInfo || [];
-
-            nodesInfoData.forEach(node => {
-              const nodeInfo = new NodeInfo(
-                node['nodeID'],
-                node['hostName'],
-                node['rackName'],
-                info['partitionName'],
-                this.formatCapacity(this.splitCapacity(node['capacity'], NOT_AVAILABLE)),
-                this.formatCapacity(this.splitCapacity(node['allocated'], NOT_AVAILABLE)),
-                this.formatCapacity(this.splitCapacity(node['occupied'], NOT_AVAILABLE)),
-                this.formatCapacity(this.splitCapacity(node['available'], NOT_AVAILABLE)),
-                []
-              );
-
-              const allocations = node['allocations'];
-              if (allocations && allocations.length > 0) {
-                const appAllocations = [];
-
-                allocations.forEach(alloc => {
-                  if (
-                    alloc.allocationTags &&
-                    alloc.allocationTags['kubernetes.io/meta/namespace'] &&
-                    alloc.allocationTags['kubernetes.io/meta/podName']
-                  ) {
-                    alloc[
-                      'displayName'
-                    ] = `${alloc.allocationTags['kubernetes.io/meta/namespace']}/${alloc.allocationTags['kubernetes.io/meta/podName']}`;
-                  } else {
-                    alloc['displayName'] = '<nil>';
-                  }
-                  appAllocations.push(
-                    new AllocationInfo(
-                      alloc['displayName'],
-                      alloc['allocationKey'],
-                      alloc['allocationTags'],
-                      alloc['uuid'],
-                      this.formatCapacity(this.splitCapacity(alloc['resource'], NOT_AVAILABLE)),
-                      alloc['priority'],
-                      alloc['queueName'],
-                      alloc['nodeId'],
-                      alloc['applicationId'],
-                      alloc['partition']
-                    )
-                  );
-                });
-
-                nodeInfo.setAllocations(appAllocations);
-              }
-
-              result.push(nodeInfo);
-            });
-          }
+          data.forEach(node => {
+            const nodeInfo = new NodeInfo(
+              node['nodeID'],
+              node['hostName'],
+              node['rackName'],
+              node['partition'] || NOT_AVAILABLE,

Review comment:
       This is the new nodes API response I got from Wilfred.
   ```
    {
         "nodeID": "node-1",
         "hostName": "",
         "rackName": "",
         "capacity": "[memory:1000 vcore:1000]",
         "allocated": "[memory:500 vcore:300]",
         "occupied": "[]",
         "available": "[memory:500 vcore:700]",
         "utilized": "[memory:50 vcore:30]",
         "allocations": [
           {
             "allocationKey": "alloc-1",
             "allocationTags": null,
             "uuid": "alloc-1-uuid",
             "resource": "[memory:500 vcore:300]",
             "priority": "0",
             "queueName": "root.default",
             "nodeId": "node-1",
             "applicationId": "app1",
             "partition": ""
           }
         ],
         "schedulable": true
       }
   ```




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

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

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