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/05/13 19:31:52 UTC

[GitHub] [accumulo] DomGarguilo opened a new pull request, #2703: WIP - Convert tables in monitor `/manager` page to DataTables

DomGarguilo opened a new pull request, #2703:
URL: https://github.com/apache/accumulo/pull/2703

   This PR moves the Manager Server table and the Log Recovery table to Datatables.
   
   Here is what the tables currently look like:
   ![image](https://user-images.githubusercontent.com/47725857/168376236-372240ff-348c-4c62-b0fc-f9761ec6db1d.png)
   
   Here is what the tables look like after these changes:
   ![Screenshot from 2022-05-13 14-56-29](https://user-images.githubusercontent.com/47725857/168376458-a9513d01-f5e0-4dac-9738-0d809e5fd3f0.png)
   
   There are a few minor differences but I just left everything as default for the time being.
   
   I am still working on figuring out why the autorefresh feature is not working to update these tables so will mark this as WIP until that is finished.
   


-- 
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] DomGarguilo commented on a diff in pull request #2703: Convert tables in monitor `/manager` page to DataTables

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on code in PR #2703:
URL: https://github.com/apache/accumulo/pull/2703#discussion_r878212471


##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/functions.js:
##########
@@ -268,6 +268,19 @@ function createEmptyRow(col, msg) {
   return '<td class="center" colspan="' + col + '"><i>' + msg + '</i></td>';
 }
 
+/**
+ * Performs an ajax reload for the given DataTable
+ *
+ * @param {DataTable} table DataTable to perform an ajax reload on
+ */
+function ajaxReloadTable(table) {
+  if (table) {
+    table.ajax.reload(null, false); // user paging is not reset on reload
+  } else {
+    console.error('There was an error reloading the given table');

Review Comment:
   I just did a quick test in the console and it would print out like this: `There was an error reloading the given table: [object Object]` since `table` is a DataTable 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.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo] DomGarguilo merged pull request #2703: Convert tables in monitor `/manager` page to DataTables

Posted by GitBox <gi...@apache.org>.
DomGarguilo merged PR #2703:
URL: https://github.com/apache/accumulo/pull/2703


-- 
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] DomGarguilo commented on pull request #2703: Convert tables in monitor `/manager` page to DataTables

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on PR #2703:
URL: https://github.com/apache/accumulo/pull/2703#issuecomment-1127836581

   I think this is now ready for review. In this PR the following was changed:
   * Made the two regular tables into DataTables
   * Refactored the tableList table into the same file as the others on the manager page
   * Applied jslint formatting
   * Added a function to reload a DataTable to functions,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] milleruntime commented on a diff in pull request #2703: Convert tables in monitor `/manager` page to DataTables

Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2703:
URL: https://github.com/apache/accumulo/pull/2703#discussion_r878242308


##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/functions.js:
##########
@@ -268,6 +268,19 @@ function createEmptyRow(col, msg) {
   return '<td class="center" colspan="' + col + '"><i>' + msg + '</i></td>';
 }
 
+/**
+ * Performs an ajax reload for the given DataTable
+ *
+ * @param {DataTable} table DataTable to perform an ajax reload on
+ */
+function ajaxReloadTable(table) {
+  if (table) {
+    table.ajax.reload(null, false); // user paging is not reset on reload
+  } else {
+    console.error('There was an error reloading the given table');

Review Comment:
   Ah yeah. I am not sure what we could print cause i think that means it doesn't exist.



-- 
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 #2703: Convert tables in monitor `/manager` page to DataTables

Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2703:
URL: https://github.com/apache/accumulo/pull/2703#discussion_r878102632


##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/functions.js:
##########
@@ -268,6 +268,19 @@ function createEmptyRow(col, msg) {
   return '<td class="center" colspan="' + col + '"><i>' + msg + '</i></td>';
 }
 
+/**
+ * Performs an ajax reload for the given DataTable
+ *
+ * @param {DataTable} table DataTable to perform an ajax reload on
+ */
+function ajaxReloadTable(table) {
+  if (table) {
+    table.ajax.reload(null, false); // user paging is not reset on reload
+  } else {
+    console.error('There was an error reloading the given table');

Review Comment:
   This is nice. You could log what was passed in.
   ```suggestion
       console.error('There was an error reloading the given table: ' + table);
   ```



-- 
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] DomGarguilo commented on a diff in pull request #2703: Convert tables in monitor `/manager` page to DataTables

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on code in PR #2703:
URL: https://github.com/apache/accumulo/pull/2703#discussion_r878245796


##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/functions.js:
##########
@@ -268,6 +268,19 @@ function createEmptyRow(col, msg) {
   return '<td class="center" colspan="' + col + '"><i>' + msg + '</i></td>';
 }
 
+/**
+ * Performs an ajax reload for the given DataTable
+ *
+ * @param {DataTable} table DataTable to perform an ajax reload on
+ */
+function ajaxReloadTable(table) {
+  if (table) {
+    table.ajax.reload(null, false); // user paging is not reset on reload
+  } else {
+    console.error('There was an error reloading the given table');

Review Comment:
   Yea not too sure. I've been looking around and haven't found a good solution so I'm thinking I'll merge this as is and we can follow up at somepoint if needed.



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