ocket8888 commented on a change in pull request #4746:
URL: https://github.com/apache/trafficcontrol/pull/4746#discussion_r487206427



##########
File path: traffic_monitor/static/script.js
##########
@@ -155,51 +155,129 @@ function getEvents() {
  * the "Cache States" table with the results - replacing the current content.
 */
 function getCacheStates() {
+       function parseIPAvailable(server, ipField) {
+               return server.status.indexOf("ONLINE") !== 0 ? server[ipField] 
: "N/A";
+       }
+
+       function parseBandwidth(server) {
+               if (Object.prototype.hasOwnProperty.call(server, 
"bandwidth_kbps")) {
+                       const kbps = (server.bandwidth_kbps / 
kilobitsInMegabit).toFixed(2);
+                       const max = 
numberStrWithCommas((server.bandwidth_capacity_kbps / 
kilobitsInMegabit).toFixed(0));

Review comment:
       This doesn't check for `bandwidth_capacity_kbps` as an own property of 
`server`, so for interfaces with no KBPS thresholds - particularly astats 
servers - it outputs max as `NaN`.

##########
File path: traffic_monitor/static/script.js
##########
@@ -155,51 +155,129 @@ function getEvents() {
  * the "Cache States" table with the results - replacing the current content.
 */
 function getCacheStates() {
+       function parseIPAvailable(server, ipField) {
+               return server.status.indexOf("ONLINE") !== 0 ? server[ipField] 
: "N/A";
+       }
+
+       function parseBandwidth(server) {
+               if (Object.prototype.hasOwnProperty.call(server, 
"bandwidth_kbps")) {
+                       const kbps = (server.bandwidth_kbps / 
kilobitsInMegabit).toFixed(2);
+                       const max = 
numberStrWithCommas((server.bandwidth_capacity_kbps / 
kilobitsInMegabit).toFixed(0));
+                       return `${kbps} / ${max}`;
+               } else {
+                       return "N/A";
+               }
+       }
+       function parseStatusClass(server, row) {
+               if (Object.prototype.hasOwnProperty.call(server, "status")) {
+                       if (server.status.indexOf("ADMIN_DOWN") !== -1 || 
server.status.indexOf("OFFLINE") !== -1) {
+                               row.classList.add("warning");
+                       } else if (!server.combined_available && 
server.status.indexOf("ONLINE") !== 0) {

Review comment:
       Interfaces have `status`, but do not have `combined_available`, 
therefore `server.combined_available` is always `undefined` for interfaces. 
Their statuses are not Traffic Ops statuses, either. They report `available` 
when they are available, not `ONLINE` or `REPORTED`.
   
   If I have a server with these interfaces:
   ```json
   {
        "interfaces": {
                "eth0":{
                        "status": "available",
                        "status_poller": "stat",
                        "bandwidth_kbps": 29,
                        "available":true
                }
        }
   }
   ```
   its row will show in text that it is available, but will be highlighted red 
as though it weren't.

##########
File path: traffic_monitor/static/script.js
##########
@@ -155,51 +155,129 @@ function getEvents() {
  * the "Cache States" table with the results - replacing the current content.
 */
 function getCacheStates() {
+       function parseIPAvailable(server, ipField) {
+               return server.status.indexOf("ONLINE") !== 0 ? server[ipField] 
: "N/A";
+       }
+
+       function parseBandwidth(server) {
+               if (Object.prototype.hasOwnProperty.call(server, 
"bandwidth_kbps")) {
+                       const kbps = (server.bandwidth_kbps / 
kilobitsInMegabit).toFixed(2);
+                       const max = 
numberStrWithCommas((server.bandwidth_capacity_kbps / 
kilobitsInMegabit).toFixed(0));

Review comment:
       This doesn't check for `bandwidth_capacity_kbps` as an own property of 
`server`, so for interfaces with no KBPS thresholds - particularly astats 
servers - it outputs max as `NaN`.

##########
File path: traffic_monitor/static/script.js
##########
@@ -155,51 +155,129 @@ function getEvents() {
  * the "Cache States" table with the results - replacing the current content.
 */
 function getCacheStates() {
+       function parseIPAvailable(server, ipField) {
+               return server.status.indexOf("ONLINE") !== 0 ? server[ipField] 
: "N/A";
+       }
+
+       function parseBandwidth(server) {
+               if (Object.prototype.hasOwnProperty.call(server, 
"bandwidth_kbps")) {
+                       const kbps = (server.bandwidth_kbps / 
kilobitsInMegabit).toFixed(2);
+                       const max = 
numberStrWithCommas((server.bandwidth_capacity_kbps / 
kilobitsInMegabit).toFixed(0));
+                       return `${kbps} / ${max}`;
+               } else {
+                       return "N/A";
+               }
+       }
+       function parseStatusClass(server, row) {
+               if (Object.prototype.hasOwnProperty.call(server, "status")) {
+                       if (server.status.indexOf("ADMIN_DOWN") !== -1 || 
server.status.indexOf("OFFLINE") !== -1) {
+                               row.classList.add("warning");
+                       } else if (!server.combined_available && 
server.status.indexOf("ONLINE") !== 0) {

Review comment:
       Interfaces have `status`, but do not have `combined_available`, 
therefore `server.combined_available` is always `undefined` for interfaces. 
Their statuses are not Traffic Ops statuses, either. They report `available` 
when they are available, not `ONLINE` or `REPORTED`.
   
   If I have a server with these interfaces:
   ```json
   {
        "interfaces": {
                "eth0":{
                        "status": "available",
                        "status_poller": "stat",
                        "bandwidth_kbps": 29,
                        "available":true
                }
        }
   }
   ```
   its row will show in text that it is available, but will be highlighted red 
as though it weren't.

##########
File path: traffic_monitor/static/script.js
##########
@@ -155,51 +155,129 @@ function getEvents() {
  * the "Cache States" table with the results - replacing the current content.
 */
 function getCacheStates() {
+       function parseIPAvailable(server, ipField) {
+               return server.status.indexOf("ONLINE") !== 0 ? server[ipField] 
: "N/A";
+       }
+
+       function parseBandwidth(server) {
+               if (Object.prototype.hasOwnProperty.call(server, 
"bandwidth_kbps")) {
+                       const kbps = (server.bandwidth_kbps / 
kilobitsInMegabit).toFixed(2);
+                       const max = 
numberStrWithCommas((server.bandwidth_capacity_kbps / 
kilobitsInMegabit).toFixed(0));

Review comment:
       This doesn't check for `bandwidth_capacity_kbps` as an own property of 
`server`, so for interfaces with no KBPS thresholds - particularly astats 
servers - it outputs max as `NaN`.

##########
File path: traffic_monitor/static/script.js
##########
@@ -155,51 +155,129 @@ function getEvents() {
  * the "Cache States" table with the results - replacing the current content.
 */
 function getCacheStates() {
+       function parseIPAvailable(server, ipField) {
+               return server.status.indexOf("ONLINE") !== 0 ? server[ipField] 
: "N/A";
+       }
+
+       function parseBandwidth(server) {
+               if (Object.prototype.hasOwnProperty.call(server, 
"bandwidth_kbps")) {
+                       const kbps = (server.bandwidth_kbps / 
kilobitsInMegabit).toFixed(2);
+                       const max = 
numberStrWithCommas((server.bandwidth_capacity_kbps / 
kilobitsInMegabit).toFixed(0));
+                       return `${kbps} / ${max}`;
+               } else {
+                       return "N/A";
+               }
+       }
+       function parseStatusClass(server, row) {
+               if (Object.prototype.hasOwnProperty.call(server, "status")) {
+                       if (server.status.indexOf("ADMIN_DOWN") !== -1 || 
server.status.indexOf("OFFLINE") !== -1) {
+                               row.classList.add("warning");
+                       } else if (!server.combined_available && 
server.status.indexOf("ONLINE") !== 0) {

Review comment:
       Interfaces have `status`, but do not have `combined_available`, 
therefore `server.combined_available` is always `undefined` for interfaces. 
Their statuses are not Traffic Ops statuses, either. They report `available` 
when they are available, not `ONLINE` or `REPORTED`.
   
   If I have a server with these interfaces:
   ```json
   {
        "interfaces": {
                "eth0":{
                        "status": "available",
                        "status_poller": "stat",
                        "bandwidth_kbps": 29,
                        "available":true
                }
        }
   }
   ```
   its row will show in text that it is available, but will be highlighted red 
as though it weren't.

##########
File path: traffic_monitor/static/script.js
##########
@@ -155,51 +155,129 @@ function getEvents() {
  * the "Cache States" table with the results - replacing the current content.
 */
 function getCacheStates() {
+       function parseIPAvailable(server, ipField) {
+               return server.status.indexOf("ONLINE") !== 0 ? server[ipField] 
: "N/A";
+       }
+
+       function parseBandwidth(server) {
+               if (Object.prototype.hasOwnProperty.call(server, 
"bandwidth_kbps")) {
+                       const kbps = (server.bandwidth_kbps / 
kilobitsInMegabit).toFixed(2);
+                       const max = 
numberStrWithCommas((server.bandwidth_capacity_kbps / 
kilobitsInMegabit).toFixed(0));

Review comment:
       This doesn't check for `bandwidth_capacity_kbps` as an own property of 
`server`, so for interfaces with no KBPS thresholds - particularly astats 
servers - it outputs max as `NaN`.

##########
File path: traffic_monitor/static/script.js
##########
@@ -155,51 +155,129 @@ function getEvents() {
  * the "Cache States" table with the results - replacing the current content.
 */
 function getCacheStates() {
+       function parseIPAvailable(server, ipField) {
+               return server.status.indexOf("ONLINE") !== 0 ? server[ipField] 
: "N/A";
+       }
+
+       function parseBandwidth(server) {
+               if (Object.prototype.hasOwnProperty.call(server, 
"bandwidth_kbps")) {
+                       const kbps = (server.bandwidth_kbps / 
kilobitsInMegabit).toFixed(2);
+                       const max = 
numberStrWithCommas((server.bandwidth_capacity_kbps / 
kilobitsInMegabit).toFixed(0));
+                       return `${kbps} / ${max}`;
+               } else {
+                       return "N/A";
+               }
+       }
+       function parseStatusClass(server, row) {
+               if (Object.prototype.hasOwnProperty.call(server, "status")) {
+                       if (server.status.indexOf("ADMIN_DOWN") !== -1 || 
server.status.indexOf("OFFLINE") !== -1) {
+                               row.classList.add("warning");
+                       } else if (!server.combined_available && 
server.status.indexOf("ONLINE") !== 0) {

Review comment:
       Interfaces have `status`, but do not have `combined_available`, 
therefore `server.combined_available` is always `undefined` for interfaces. 
Their statuses are not Traffic Ops statuses, either. They report `available` 
when they are available, not `ONLINE` or `REPORTED`.
   
   If I have a server with these interfaces:
   ```json
   {
        "interfaces": {
                "eth0":{
                        "status": "available",
                        "status_poller": "stat",
                        "bandwidth_kbps": 29,
                        "available":true
                }
        }
   }
   ```
   its row will show in text that it is available, but will be highlighted red 
as though it weren't.

##########
File path: traffic_monitor/static/script.js
##########
@@ -155,51 +155,129 @@ function getEvents() {
  * the "Cache States" table with the results - replacing the current content.
 */
 function getCacheStates() {
+       function parseIPAvailable(server, ipField) {
+               return server.status.indexOf("ONLINE") !== 0 ? server[ipField] 
: "N/A";
+       }
+
+       function parseBandwidth(server) {
+               if (Object.prototype.hasOwnProperty.call(server, 
"bandwidth_kbps")) {
+                       const kbps = (server.bandwidth_kbps / 
kilobitsInMegabit).toFixed(2);
+                       const max = 
numberStrWithCommas((server.bandwidth_capacity_kbps / 
kilobitsInMegabit).toFixed(0));

Review comment:
       This doesn't check for `bandwidth_capacity_kbps` as an own property of 
`server`, so for interfaces with no KBPS thresholds - particularly astats 
servers - it outputs max as `NaN`.

##########
File path: traffic_monitor/static/script.js
##########
@@ -155,51 +155,129 @@ function getEvents() {
  * the "Cache States" table with the results - replacing the current content.
 */
 function getCacheStates() {
+       function parseIPAvailable(server, ipField) {
+               return server.status.indexOf("ONLINE") !== 0 ? server[ipField] 
: "N/A";
+       }
+
+       function parseBandwidth(server) {
+               if (Object.prototype.hasOwnProperty.call(server, 
"bandwidth_kbps")) {
+                       const kbps = (server.bandwidth_kbps / 
kilobitsInMegabit).toFixed(2);
+                       const max = 
numberStrWithCommas((server.bandwidth_capacity_kbps / 
kilobitsInMegabit).toFixed(0));
+                       return `${kbps} / ${max}`;
+               } else {
+                       return "N/A";
+               }
+       }
+       function parseStatusClass(server, row) {
+               if (Object.prototype.hasOwnProperty.call(server, "status")) {
+                       if (server.status.indexOf("ADMIN_DOWN") !== -1 || 
server.status.indexOf("OFFLINE") !== -1) {
+                               row.classList.add("warning");
+                       } else if (!server.combined_available && 
server.status.indexOf("ONLINE") !== 0) {

Review comment:
       Interfaces have `status`, but do not have `combined_available`, 
therefore `server.combined_available` is always `undefined` for interfaces. 
Their statuses are not Traffic Ops statuses, either. They report `available` 
when they are available, not `ONLINE` or `REPORTED`.
   
   If I have a server with these interfaces:
   ```json
   {
        "interfaces": {
                "eth0":{
                        "status": "available",
                        "status_poller": "stat",
                        "bandwidth_kbps": 29,
                        "available":true
                }
        }
   }
   ```
   its row will show in text that it is available, but will be highlighted red 
as though it weren't.




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