You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2022/04/27 12:02:48 UTC
[GitHub] [accumulo] milleruntime opened a new pull request, #2652: Replace more js with jquery in server.js
milleruntime opened a new pull request, #2652:
URL: https://github.com/apache/accumulo/pull/2652
* Replace javascript code with more readable and clearer jquery
* Add 3 static rows to tserver table instead of dynamically creating the rows
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on a diff in pull request #2652: Replace more js with jquery in server.js
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2652:
URL: https://github.com/apache/accumulo/pull/2652#discussion_r859745308
##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/server.js:
##########
@@ -61,67 +61,60 @@ function refreshDetailTable() {
* Generates the server history table
*/
function refreshHistoryTable() {
-
- clearTableBody('opHistoryDetails');
-
var data = sessionStorage.server === undefined ?
[] : JSON.parse(sessionStorage.server);
if (data.length === 0 || data.allTimeTabletResults === undefined) {
- var row = [];
-
- row.push(createEmptyRow(8, 'Empty'));
-
- $('<tr/>', {
- html: row.join('')
- }).appendTo('#opHistoryDetails tbody');
+ $("#opHistoryDetails > tbody > tr > td").each(function () {
+ $(this).text(0);
+ });
Review Comment:
Yeah that is probably better.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on a diff in pull request #2652: Replace more js with jquery in server.js
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2652:
URL: https://github.com/apache/accumulo/pull/2652#discussion_r859712479
##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/server.js:
##########
@@ -61,67 +61,60 @@ function refreshDetailTable() {
* Generates the server history table
*/
function refreshHistoryTable() {
-
- clearTableBody('opHistoryDetails');
-
var data = sessionStorage.server === undefined ?
[] : JSON.parse(sessionStorage.server);
if (data.length === 0 || data.allTimeTabletResults === undefined) {
- var row = [];
-
- row.push(createEmptyRow(8, 'Empty'));
-
- $('<tr/>', {
- html: row.join('')
- }).appendTo('#opHistoryDetails tbody');
+ $("#opHistoryDetails > tbody > tr > td").each(function () {
+ $(this).text(0);
+ });
Review Comment:
This puts 0 in every cell. I think its OK since this is only if no data comes back from the REST call. But I am not sure of when this actually happens.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime merged pull request #2652: Replace more js with jquery in server.js
Posted by GitBox <gi...@apache.org>.
milleruntime merged PR #2652:
URL: https://github.com/apache/accumulo/pull/2652
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on a diff in pull request #2652: Replace more js with jquery in server.js
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2652:
URL: https://github.com/apache/accumulo/pull/2652#discussion_r860947602
##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/server.js:
##########
@@ -58,115 +57,80 @@ function refreshDetailTable() {
}
/**
- * Generates the server history table
+ * Populates the All Time Tablet Operations table
*/
function refreshHistoryTable() {
-
- clearTableBody('opHistoryDetails');
-
var data = sessionStorage.server === undefined ?
[] : JSON.parse(sessionStorage.server);
if (data.length === 0 || data.allTimeTabletResults === undefined) {
- var row = [];
-
- row.push(createEmptyRow(8, 'Empty'));
-
- $('<tr/>', {
- html: row.join('')
- }).appendTo('#opHistoryDetails tbody');
+ clearAllTableCells("opHistoryDetails");
} else {
var totalTimeSpent = 0;
$.each(data.allTimeTabletResults, function(key, val) {
totalTimeSpent += val.timeSpent;
});
$.each(data.allTimeTabletResults, function(key, val) {
- var row = [];
-
- row.push(createFirstCell(val.operation, val.operation));
-
- row.push(createRightCell(val.success, bigNumberForQuantity(val.success)));
-
- row.push(createRightCell(val.failure,
- bigNumberForQuantity(val.failure)));
-
- row.push(createRightCell((val.avgQueueTime == null ?
- '-' : val.avgQueueTime * 1000.0),
- (val.avgQueueTime == null ?
- '—' : timeDuration(val.avgQueueTime * 1000.0))));
-
- row.push(createRightCell((val.queueStdDev == null ?
- '-' : val.queueStdDev * 1000.0),
- (val.queueStdDev == null ?
- '—' : timeDuration(val.queueStdDev * 1000.0))));
-
- row.push(createRightCell((val.avgTime == null ?
- '-' : val.avgTime * 1000.0),
- (val.avgTime == null ?
- '—' : timeDuration(val.avgTime * 1000.0))));
-
- row.push(createRightCell((val.stdDev == null ?
- '-' : val.stdDev * 1000.0),
- (val.stdDev == null ?
- '—' : timeDuration(val.stdDev * 1000.0))));
-
- row.push(createRightCell(((val.timeSpent / totalTimeSpent) * 100),
- '<div class="progress"><div class="progress-bar"' +
- ' role="progressbar" style="min-width: 2em; width:' +
- Math.floor((val.timeSpent / totalTimeSpent) * 100) +
- '%;">' + Math.floor((val.timeSpent / totalTimeSpent) * 100) +
- '%</div></div>'));
-
- $('<tr/>', {
- html: row.join('')
- }).appendTo('#opHistoryDetails tbody');
-
+ // use the first 5 characters of the operation for the jquery selector
+ var rowId = "#" + val.operation.slice(0, 5) + "Row";
+ console.log("Populating rowId " + rowId + " with " + val.operation + " data");
+
+ $(rowId).append("<td>" + val.operation + "</td>");
+ $(rowId).append("<td>" + bigNumberForQuantity(val.success) + "</td>");
+ $(rowId).append("<td>" + bigNumberForQuantity(val.failure) + "</td>");
+
+ if (val.avgQueueTime == null) {
+ $(rowId).append(dashCell);
+ } else {
+ $(rowId).append("<td>" + timeDuration(val.avgQueueTime * 1000.0) + "</td>");
+ }
Review Comment:
Added new method in cf930c4d5f75fd2f01b48d55dcc21edda8e49f49
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on pull request #2652: Replace more js with jquery in server.js
Posted by GitBox <gi...@apache.org>.
milleruntime commented on PR #2652:
URL: https://github.com/apache/accumulo/pull/2652#issuecomment-1110950184
@ctubbsii I want to do the other tables on this page as well. If you want to hold off reviewing this, I can convert it to a draft and clean up the entire `server.js`.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a diff in pull request #2652: Replace more js with jquery in server.js
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on code in PR #2652:
URL: https://github.com/apache/accumulo/pull/2652#discussion_r859743294
##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/server.js:
##########
@@ -61,67 +61,60 @@ function refreshDetailTable() {
* Generates the server history table
*/
function refreshHistoryTable() {
-
- clearTableBody('opHistoryDetails');
-
var data = sessionStorage.server === undefined ?
[] : JSON.parse(sessionStorage.server);
if (data.length === 0 || data.allTimeTabletResults === undefined) {
- var row = [];
-
- row.push(createEmptyRow(8, 'Empty'));
-
- $('<tr/>', {
- html: row.join('')
- }).appendTo('#opHistoryDetails tbody');
+ $("#opHistoryDetails > tbody > tr > td").each(function () {
+ $(this).text(0);
+ });
Review Comment:
Should you just put `""` then?
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a diff in pull request #2652: Replace more js with jquery in server.js
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on code in PR #2652:
URL: https://github.com/apache/accumulo/pull/2652#discussion_r860751392
##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/functions.js:
##########
@@ -616,3 +616,13 @@ function getStatus() {
sessionStorage.status = JSON.stringify(data);
});
}
+
+/*
+ * Jquery call to clear all data from cells of a table
+ */
+function clearAllTableCells(tableId) {
+ console.log("Clearing all table cell data for " + tableId);
+ $("#" + tableId + " > tbody > tr > td").each(function () {
+ $(this).text("");
+ });
+}
Review Comment:
File is missing a terminating newline.
```suggestion
}
```
##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/server.js:
##########
@@ -58,115 +57,80 @@ function refreshDetailTable() {
}
/**
- * Generates the server history table
+ * Populates the All Time Tablet Operations table
*/
function refreshHistoryTable() {
-
- clearTableBody('opHistoryDetails');
-
var data = sessionStorage.server === undefined ?
[] : JSON.parse(sessionStorage.server);
if (data.length === 0 || data.allTimeTabletResults === undefined) {
- var row = [];
-
- row.push(createEmptyRow(8, 'Empty'));
-
- $('<tr/>', {
- html: row.join('')
- }).appendTo('#opHistoryDetails tbody');
+ clearAllTableCells("opHistoryDetails");
} else {
var totalTimeSpent = 0;
$.each(data.allTimeTabletResults, function(key, val) {
totalTimeSpent += val.timeSpent;
});
$.each(data.allTimeTabletResults, function(key, val) {
- var row = [];
-
- row.push(createFirstCell(val.operation, val.operation));
-
- row.push(createRightCell(val.success, bigNumberForQuantity(val.success)));
-
- row.push(createRightCell(val.failure,
- bigNumberForQuantity(val.failure)));
-
- row.push(createRightCell((val.avgQueueTime == null ?
- '-' : val.avgQueueTime * 1000.0),
- (val.avgQueueTime == null ?
- '—' : timeDuration(val.avgQueueTime * 1000.0))));
-
- row.push(createRightCell((val.queueStdDev == null ?
- '-' : val.queueStdDev * 1000.0),
- (val.queueStdDev == null ?
- '—' : timeDuration(val.queueStdDev * 1000.0))));
-
- row.push(createRightCell((val.avgTime == null ?
- '-' : val.avgTime * 1000.0),
- (val.avgTime == null ?
- '—' : timeDuration(val.avgTime * 1000.0))));
-
- row.push(createRightCell((val.stdDev == null ?
- '-' : val.stdDev * 1000.0),
- (val.stdDev == null ?
- '—' : timeDuration(val.stdDev * 1000.0))));
-
- row.push(createRightCell(((val.timeSpent / totalTimeSpent) * 100),
- '<div class="progress"><div class="progress-bar"' +
- ' role="progressbar" style="min-width: 2em; width:' +
- Math.floor((val.timeSpent / totalTimeSpent) * 100) +
- '%;">' + Math.floor((val.timeSpent / totalTimeSpent) * 100) +
- '%</div></div>'));
-
- $('<tr/>', {
- html: row.join('')
- }).appendTo('#opHistoryDetails tbody');
-
+ // use the first 5 characters of the operation for the jquery selector
+ var rowId = "#" + val.operation.slice(0, 5) + "Row";
+ console.log("Populating rowId " + rowId + " with " + val.operation + " data");
+
+ $(rowId).append("<td>" + val.operation + "</td>");
+ $(rowId).append("<td>" + bigNumberForQuantity(val.success) + "</td>");
+ $(rowId).append("<td>" + bigNumberForQuantity(val.failure) + "</td>");
+
+ if (val.avgQueueTime == null) {
+ $(rowId).append(dashCell);
+ } else {
+ $(rowId).append("<td>" + timeDuration(val.avgQueueTime * 1000.0) + "</td>");
+ }
Review Comment:
This logic is repeated a lot. Could put this in a separate function:
```js
function appendDurationToCell(rowId, value) {
let v = dashCell;
if (value != null) {
v = "<td>" + timeDuration(val.avgQueueTime * 1000.0) + "</td>";
}
$(rowId).append(v);
}
// ...
appendDurationToCell(rowId, val.avgQueueTime);
appendDurationToCell(rowId, val.queueStdDev);
```
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org