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/23 20:29:43 UTC
[GitHub] [accumulo] DomGarguilo opened a new pull request, #2725: WIP - Convert monitor tables in server.js to DataTables
DomGarguilo opened a new pull request, #2725:
URL: https://github.com/apache/accumulo/pull/2725
The changes in this PR convert the tables in server.js (which corresponds to the per server pages at http://localhost:9995/tservers?s=hostname) in the monitor.
--
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 #2725: Convert monitor tables in server.js to DataTables
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on code in PR #2725:
URL: https://github.com/apache/accumulo/pull/2725#discussion_r888930484
##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/server.js:
##########
@@ -18,18 +18,19 @@
*/
"use strict";
-var serv;
-var tabletResults;
+var detailTable, historyTable, currentTable, resultsTable;
+
+const url = '/rest/tservers/' + window.location.search.split('=')[1];
Review Comment:
This is just the first method that worked for me. I need the hostname for the server and this is one way I figured out how to do it (grab it from the current page url).
--
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 #2725: Convert monitor tables in server.js to DataTables
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2725:
URL: https://github.com/apache/accumulo/pull/2725#discussion_r888836205
##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/server.js:
##########
@@ -18,18 +18,19 @@
*/
"use strict";
-var serv;
-var tabletResults;
+var detailTable, historyTable, currentTable, resultsTable;
+
+const url = '/rest/tservers/' + window.location.search.split('=')[1];
Review Comment:
Why get the URL this way? I worry if this is compatible with older browsers.
--
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 #2725: WIP - Convert monitor tables in server.js to DataTables
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on PR #2725:
URL: https://github.com/apache/accumulo/pull/2725#issuecomment-1135109520
I marked this as WIP because there is an error being thrown that I am currently trying to fix.
--
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 #2725: Convert monitor tables in server.js to DataTables
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on PR #2725:
URL: https://github.com/apache/accumulo/pull/2725#issuecomment-1145329896
This PR is now ready for review. Everything seems to be working as expected.
Here is a screenshot before these changes:
And here is a screenshot after these changes:
*Note: The empty cells are expected. There is not always data available for every cell.
--
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 #2725: Convert monitor tables in server.js to DataTables
Posted by GitBox <gi...@apache.org>.
DomGarguilo merged PR #2725:
URL: https://github.com/apache/accumulo/pull/2725
--
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 pull request #2725: WIP - Convert monitor tables in server.js to DataTables
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on PR #2725:
URL: https://github.com/apache/accumulo/pull/2725#issuecomment-1135223672
I merged main onto this and resolved the conflicts caused by #2720 . Based on last comment, though, I'm not sure this is ready for review/merging.
--
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 #2725: Convert monitor tables in server.js to DataTables
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on code in PR #2725:
URL: https://github.com/apache/accumulo/pull/2725#discussion_r889036684
##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/server.js:
##########
@@ -18,18 +18,19 @@
*/
"use strict";
-var serv;
-var tabletResults;
+var detailTable, historyTable, currentTable, resultsTable;
+
+const url = '/rest/tservers/' + window.location.search.split('=')[1];
Review Comment:
I wasn't quite able to get this working without explicitly passing the variable into the function that initializes the DataTables so that's what I ended up doing. Check out [be1fc59](https://github.com/apache/accumulo/pull/2725/commits/be1fc59b182d087970e8739c1563da4bcf62a818) and let me know if that seems like a fine approach to you.
--
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 #2725: Convert monitor tables in server.js to DataTables
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on code in PR #2725:
URL: https://github.com/apache/accumulo/pull/2725#discussion_r888930484
##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/server.js:
##########
@@ -18,18 +18,19 @@
*/
"use strict";
-var serv;
-var tabletResults;
+var detailTable, historyTable, currentTable, resultsTable;
+
+const url = '/rest/tservers/' + window.location.search.split('=')[1];
Review Comment:
This is just the first method that worked for me. I need the hostname for the server and this is one way I figured out how to do it (grab it from the current page url). There is probably a better way to do this. Any ideas?
--
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 #2725: Convert monitor tables in server.js to DataTables
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2725:
URL: https://github.com/apache/accumulo/pull/2725#discussion_r888951009
##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/server.js:
##########
@@ -18,18 +18,19 @@
*/
"use strict";
-var serv;
-var tabletResults;
+var detailTable, historyTable, currentTable, resultsTable;
+
+const url = '/rest/tservers/' + window.location.search.split('=')[1];
Review Comment:
Currently we are using the template `server.ftl` to get the hostname from the server when it generates the page using the freemarker syntax:
<pre>
// Global constant for the page
serv = '${server}';
</pre>
You should still be able to do that with a constant.
--
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 #2725: WIP - Convert monitor tables in server.js to DataTables
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on PR #2725:
URL: https://github.com/apache/accumulo/pull/2725#issuecomment-1135914922
> I merged main onto this and resolved the conflicts caused by #2720 .
Thanks!
> Based on last comment, though, I'm not sure this is ready for review/merging.
That is correct, this is not ready yet,
--
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