You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "fapifta (via GitHub)" <gi...@apache.org> on 2023/02/22 00:45:20 UTC

[GitHub] [ozone] fapifta commented on a diff in pull request #4289: HDDS-7816. Add DataNode list to the SCM WebUI

fapifta commented on code in PR #4289:
URL: https://github.com/apache/ozone/pull/4289#discussion_r1113655293


##########
hadoop-hdds/server-scm/src/main/resources/webapps/scm/scm.js:
##########
@@ -24,12 +24,55 @@
         require: {
             overview: "^overview"
         },
-        controller: function ($http) {
+        controller: function ($http,$scope) {
             var ctrl = this;
+            $scope.reverse = true;
+            $scope.columnName = "";
+            let nodeStatusCopy = [];
+            $scope.RecordsToDisplay = "10";
+            $scope.currentPage = 1;
+            $scope.lastIndex = 0;
+
             $http.get("jmx?qry=Hadoop:service=SCMNodeManager,name=SCMNodeManagerInfo")
                 .then(function (result) {
                     ctrl.nodemanagermetrics = result.data.beans[0];
-                });
+                $scope.nodeStatus = ctrl.nodemanagermetrics && ctrl.nodemanagermetrics.NodeStatusInfo && ctrl.nodemanagermetrics.NodeStatusInfo.map(({ key, value}) =>{
+                            return {
+                                hostname: key,
+                                opstate: value[0],
+                                comstate: value[1]
+                            }});
+                nodeStatusCopy = [...$scope.nodeStatus];
+                $scope.lastIndex = Math.ceil(nodeStatusCopy.length/$scope.RecordsToDisplay);
+                $scope.nodeStatus = nodeStatusCopy.slice(0, $scope.RecordsToDisplay);
+                  });

Review Comment:
   This line should be 2 spaces to the left to have the correct indentation, as this belongs to the .then... in line 37, so it should be on the same level.



##########
hadoop-hdds/server-scm/src/main/resources/webapps/scm/scm.js:
##########
@@ -24,12 +24,55 @@
         require: {
             overview: "^overview"
         },
-        controller: function ($http) {
+        controller: function ($http,$scope) {
             var ctrl = this;
+            $scope.reverse = true;
+            $scope.columnName = "";
+            let nodeStatusCopy = [];
+            $scope.RecordsToDisplay = "10";
+            $scope.currentPage = 1;
+            $scope.lastIndex = 0;
+
             $http.get("jmx?qry=Hadoop:service=SCMNodeManager,name=SCMNodeManagerInfo")
                 .then(function (result) {
                     ctrl.nodemanagermetrics = result.data.beans[0];
-                });
+                $scope.nodeStatus = ctrl.nodemanagermetrics && ctrl.nodemanagermetrics.NodeStatusInfo && ctrl.nodemanagermetrics.NodeStatusInfo.map(({ key, value}) =>{
+                            return {
+                                hostname: key,
+                                opstate: value[0],
+                                comstate: value[1]
+                            }});
+                nodeStatusCopy = [...$scope.nodeStatus];
+                $scope.lastIndex = Math.ceil(nodeStatusCopy.length/$scope.RecordsToDisplay);
+                $scope.nodeStatus = nodeStatusCopy.slice(0, $scope.RecordsToDisplay);
+                  });
+                /*if option is 'All' display all records else display specified record on page*/
+                $scope.UpdateRecordsToShow = () => {
+                     if($scope.RecordsToDisplay == 'All') {
+                         $scope.lastIndex = 1;
+                         $scope.nodeStatus = nodeStatusCopy;
+                     }
+                     else {
+                         $scope.lastIndex = Math.ceil(nodeStatusCopy.length/$scope.RecordsToDisplay);

Review Comment:
   Missing space around the / operator.



##########
hadoop-hdds/server-scm/src/main/resources/webapps/scm/scm.js:
##########
@@ -24,12 +24,55 @@
         require: {
             overview: "^overview"
         },
-        controller: function ($http) {
+        controller: function ($http,$scope) {
             var ctrl = this;
+            $scope.reverse = true;
+            $scope.columnName = "";
+            let nodeStatusCopy = [];
+            $scope.RecordsToDisplay = "10";
+            $scope.currentPage = 1;
+            $scope.lastIndex = 0;
+
             $http.get("jmx?qry=Hadoop:service=SCMNodeManager,name=SCMNodeManagerInfo")
                 .then(function (result) {
                     ctrl.nodemanagermetrics = result.data.beans[0];
-                });
+                $scope.nodeStatus = ctrl.nodemanagermetrics && ctrl.nodemanagermetrics.NodeStatusInfo && ctrl.nodemanagermetrics.NodeStatusInfo.map(({ key, value}) =>{
+                            return {
+                                hostname: key,
+                                opstate: value[0],
+                                comstate: value[1]
+                            }});
+                nodeStatusCopy = [...$scope.nodeStatus];
+                $scope.lastIndex = Math.ceil(nodeStatusCopy.length/$scope.RecordsToDisplay);
+                $scope.nodeStatus = nodeStatusCopy.slice(0, $scope.RecordsToDisplay);
+                  });
+                /*if option is 'All' display all records else display specified record on page*/

Review Comment:
   for this line to up until line 75 (inclusive), every line should be 4 spaces to the left, while the handle pagination part by 5 spaces, as that is indented 1 to the right now even compared to this line.



##########
hadoop-hdds/server-scm/src/main/resources/webapps/scm/scm.js:
##########
@@ -24,12 +24,55 @@
         require: {
             overview: "^overview"
         },
-        controller: function ($http) {
+        controller: function ($http,$scope) {
             var ctrl = this;
+            $scope.reverse = true;
+            $scope.columnName = "";
+            let nodeStatusCopy = [];
+            $scope.RecordsToDisplay = "10";
+            $scope.currentPage = 1;
+            $scope.lastIndex = 0;
+
             $http.get("jmx?qry=Hadoop:service=SCMNodeManager,name=SCMNodeManagerInfo")
                 .then(function (result) {
                     ctrl.nodemanagermetrics = result.data.beans[0];
-                });
+                $scope.nodeStatus = ctrl.nodemanagermetrics && ctrl.nodemanagermetrics.NodeStatusInfo && ctrl.nodemanagermetrics.NodeStatusInfo.map(({ key, value}) =>{
+                            return {
+                                hostname: key,
+                                opstate: value[0],
+                                comstate: value[1]
+                            }});
+                nodeStatusCopy = [...$scope.nodeStatus];
+                $scope.lastIndex = Math.ceil(nodeStatusCopy.length/$scope.RecordsToDisplay);
+                $scope.nodeStatus = nodeStatusCopy.slice(0, $scope.RecordsToDisplay);
+                  });
+                /*if option is 'All' display all records else display specified record on page*/
+                $scope.UpdateRecordsToShow = () => {
+                     if($scope.RecordsToDisplay == 'All') {
+                         $scope.lastIndex = 1;
+                         $scope.nodeStatus = nodeStatusCopy;
+                     }
+                     else {

Review Comment:
   This else { should go to the previous line.



##########
hadoop-hdds/server-scm/src/main/resources/webapps/scm/scm.js:
##########
@@ -24,12 +24,55 @@
         require: {
             overview: "^overview"
         },
-        controller: function ($http) {
+        controller: function ($http,$scope) {
             var ctrl = this;
+            $scope.reverse = true;
+            $scope.columnName = "";
+            let nodeStatusCopy = [];
+            $scope.RecordsToDisplay = "10";
+            $scope.currentPage = 1;
+            $scope.lastIndex = 0;
+
             $http.get("jmx?qry=Hadoop:service=SCMNodeManager,name=SCMNodeManagerInfo")
                 .then(function (result) {
                     ctrl.nodemanagermetrics = result.data.beans[0];
-                });
+                $scope.nodeStatus = ctrl.nodemanagermetrics && ctrl.nodemanagermetrics.NodeStatusInfo && ctrl.nodemanagermetrics.NodeStatusInfo.map(({ key, value}) =>{

Review Comment:
   This line, and the following line up until line 47 (inclusive) should be 4 spaces to the right to have the correct indentation level.
   Also in this line at the end there is a missing space between value and }, and => and {.



##########
hadoop-hdds/server-scm/src/main/resources/webapps/scm/scm.js:
##########
@@ -24,12 +24,55 @@
         require: {
             overview: "^overview"
         },
-        controller: function ($http) {
+        controller: function ($http,$scope) {
             var ctrl = this;
+            $scope.reverse = true;
+            $scope.columnName = "";
+            let nodeStatusCopy = [];
+            $scope.RecordsToDisplay = "10";
+            $scope.currentPage = 1;
+            $scope.lastIndex = 0;
+
             $http.get("jmx?qry=Hadoop:service=SCMNodeManager,name=SCMNodeManagerInfo")
                 .then(function (result) {
                     ctrl.nodemanagermetrics = result.data.beans[0];
-                });
+                $scope.nodeStatus = ctrl.nodemanagermetrics && ctrl.nodemanagermetrics.NodeStatusInfo && ctrl.nodemanagermetrics.NodeStatusInfo.map(({ key, value}) =>{
+                            return {
+                                hostname: key,
+                                opstate: value[0],
+                                comstate: value[1]
+                            }});
+                nodeStatusCopy = [...$scope.nodeStatus];
+                $scope.lastIndex = Math.ceil(nodeStatusCopy.length/$scope.RecordsToDisplay);

Review Comment:
   Missing space around the / operator.



##########
hadoop-hdds/server-scm/src/main/resources/webapps/scm/scm.js:
##########
@@ -24,12 +24,55 @@
         require: {
             overview: "^overview"
         },
-        controller: function ($http) {
+        controller: function ($http,$scope) {
             var ctrl = this;
+            $scope.reverse = true;
+            $scope.columnName = "";
+            let nodeStatusCopy = [];
+            $scope.RecordsToDisplay = "10";
+            $scope.currentPage = 1;
+            $scope.lastIndex = 0;
+
             $http.get("jmx?qry=Hadoop:service=SCMNodeManager,name=SCMNodeManagerInfo")
                 .then(function (result) {
                     ctrl.nodemanagermetrics = result.data.beans[0];
-                });
+                $scope.nodeStatus = ctrl.nodemanagermetrics && ctrl.nodemanagermetrics.NodeStatusInfo && ctrl.nodemanagermetrics.NodeStatusInfo.map(({ key, value}) =>{
+                            return {
+                                hostname: key,
+                                opstate: value[0],
+                                comstate: value[1]
+                            }});
+                nodeStatusCopy = [...$scope.nodeStatus];
+                $scope.lastIndex = Math.ceil(nodeStatusCopy.length/$scope.RecordsToDisplay);
+                $scope.nodeStatus = nodeStatusCopy.slice(0, $scope.RecordsToDisplay);
+                  });
+                /*if option is 'All' display all records else display specified record on page*/
+                $scope.UpdateRecordsToShow = () => {
+                     if($scope.RecordsToDisplay == 'All') {
+                         $scope.lastIndex = 1;
+                         $scope.nodeStatus = nodeStatusCopy;
+                     }
+                     else {
+                         $scope.lastIndex = Math.ceil(nodeStatusCopy.length/$scope.RecordsToDisplay);
+                         $scope.nodeStatus = nodeStatusCopy.slice(0, $scope.RecordsToDisplay);
+                     }
+                     $scope.currentPage=1;

Review Comment:
   Missing spaces around the = sign



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeManagerMXBean.java:
##########
@@ -41,5 +42,6 @@ public interface NodeManagerMXBean {
    * storage type.
    */
   Map<String, Long> getNodeInfo();
+  Map<String, List<String>> getNodeStatusInfo();

Review Comment:
   Please add some API documentation for this method, to describe what is being generated and returned by the method.



-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org