[GitHub] storm pull request #2505: STORM-2877: Add an option to configure pagination ...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/storm/pull/2505


---

[GitHub] storm pull request #2505: STORM-2877: Add an option to configure pagination ...

[srishtyagrawal]

Github user srishtyagrawal commented on a diff in the pull request:

https://github.com/apache/storm/pull/2505#discussion_r160776471
  
--- Diff: storm-core/src/ui/public/index.html ---
@@ -106,92 +106,92 @@ Nimbus Configuration
 $.blockUI({ message: ' Loading 
summary...'});
 });
 $(document).ready(function() {
-$.extend( $.fn.dataTable.defaults, {
-  stateSave: true,
-  lengthMenu: [[20,40,60,100,-1], [20, 40, 60, 100, "All"]],
-  pageLength: 20
-});
-
-$.ajaxSetup({
-"error":function(jqXHR,textStatus,response) {
-var errorJson = jQuery.parseJSON(jqXHR.responseText);
-getStatic("/templates/json-error-template.html", 
function(template) {
-
$("#json-response-error").append(Mustache.render($(template).filter("#json-error-template").html(),errorJson));
-});
-}
-});
-var uiUser = $("#ui-user");
-var clusterSummary = $("#cluster-summary");
-var clusterResources = $("#cluster-resources");
-var nimbusSummary = $("#nimbus-summary");
-var ownerSummary = $("#owner-summary");
-var topologySummary = $("#topology-summary");
-var supervisorSummary = $("#supervisor-summary");
-var config = $("#nimbus-configuration");
-
-getStatic("/templates/index-page-template.html", 
function(indexTemplate) {
-
$.getJSON("/api/v1/cluster/summary",function(response,status,jqXHR) {
-getStatic("/templates/user-template.html", function(template) {
-
uiUser.append(Mustache.render($(template).filter("#user-template").html(),response));
-$('#ui-user [data-toggle="tooltip"]').tooltip()
-});
-
-
clusterSummary.append(Mustache.render($(indexTemplate).filter("#cluster-summary-template").html(),response));
-$('#cluster-summary [data-toggle="tooltip"]').tooltip();
-
-
clusterResources.append(Mustache.render($(indexTemplate).filter("#cluster-resources-template").html(),response));
-$('#cluster-resources [data-toggle="tooltip"]').tooltip();
+
$.getJSON("/api/v1/cluster/configuration",function(response,status,jqXHR) {
--- End diff --

Done!


---

[GitHub] storm pull request #2505: STORM-2877: Add an option to configure pagination ...

[revans2]

Github user revans2 commented on a diff in the pull request:

https://github.com/apache/storm/pull/2505#discussion_r160736089
  
--- Diff: storm-core/src/ui/public/index.html ---
@@ -106,92 +106,92 @@ Nimbus Configuration
 $.blockUI({ message: ' Loading 
summary...'});
 });
 $(document).ready(function() {
-$.extend( $.fn.dataTable.defaults, {
-  stateSave: true,
-  lengthMenu: [[20,40,60,100,-1], [20, 40, 60, 100, "All"]],
-  pageLength: 20
-});
-
-$.ajaxSetup({
-"error":function(jqXHR,textStatus,response) {
-var errorJson = jQuery.parseJSON(jqXHR.responseText);
-getStatic("/templates/json-error-template.html", 
function(template) {
-
$("#json-response-error").append(Mustache.render($(template).filter("#json-error-template").html(),errorJson));
-});
-}
-});
-var uiUser = $("#ui-user");
-var clusterSummary = $("#cluster-summary");
-var clusterResources = $("#cluster-resources");
-var nimbusSummary = $("#nimbus-summary");
-var ownerSummary = $("#owner-summary");
-var topologySummary = $("#topology-summary");
-var supervisorSummary = $("#supervisor-summary");
-var config = $("#nimbus-configuration");
-
-getStatic("/templates/index-page-template.html", 
function(indexTemplate) {
-
$.getJSON("/api/v1/cluster/summary",function(response,status,jqXHR) {
-getStatic("/templates/user-template.html", function(template) {
-
uiUser.append(Mustache.render($(template).filter("#user-template").html(),response));
-$('#ui-user [data-toggle="tooltip"]').tooltip()
-});
-
-
clusterSummary.append(Mustache.render($(indexTemplate).filter("#cluster-summary-template").html(),response));
-$('#cluster-summary [data-toggle="tooltip"]').tooltip();
-
-
clusterResources.append(Mustache.render($(indexTemplate).filter("#cluster-resources-template").html(),response));
-$('#cluster-resources [data-toggle="tooltip"]').tooltip();
+
$.getJSON("/api/v1/cluster/configuration",function(response,status,jqXHR) {
--- End diff --

nit: Could we rename this configuration response and status?  They are 
over-ridden by the other ajax calls and I would prefer that it is not confusing 
in the code which variable is being accessed at which time.


---

[GitHub] storm pull request #2505: STORM-2877: Add an option to configure pagination ...

[srishtyagrawal]

Github user srishtyagrawal commented on a diff in the pull request:

https://github.com/apache/storm/pull/2505#discussion_r160570956
  
--- Diff: storm-server/src/main/java/org/apache/storm/DaemonConfig.java ---
@@ -307,6 +307,12 @@
 @isString
 public static final String UI_CENTRAL_LOGGING_URL = 
"ui.central.logging.url";
 
+/**
+ * Storm UI drop-down pagination value.
--- End diff --

Done!


---

[GitHub] storm pull request #2505: STORM-2877: Add an option to configure pagination ...

[revans2]

Github user revans2 commented on a diff in the pull request:

https://github.com/apache/storm/pull/2505#discussion_r160239743
  
--- Diff: storm-server/src/main/java/org/apache/storm/DaemonConfig.java ---
@@ -307,6 +307,12 @@
 @isString
 public static final String UI_CENTRAL_LOGGING_URL = 
"ui.central.logging.url";
 
+/**
+ * Storm UI drop-down pagination value.
--- End diff --

Could you also add in some example values that can be set in the javadocs.  
Just so people know what to do.


---

[GitHub] storm pull request #2505: STORM-2877: Add an option to configure pagination ...

[srishtyagrawal]

GitHub user srishtyagrawal opened a pull request:

https://github.com/apache/storm/pull/2505

STORM-2877: Add an option to configure pagination in Storm UI

The current pagination default value for Storm UI is hard-coded to be 20. 
Pagination has been introduced in Storm 1.x. Having 20 items in the list 
restricts searching through the page. It will be beneficial to have a 
configuration option, `ui.pagination`, which will set the pagination value when 
Storm UI loads. This option can be added to storm.yaml along with other 
configurations.

Changed the value of `ui.pagination` in `storm.yaml` to test the following 
cases :



| Value in `storm.yaml`| Result
| - |:-:| 
|  No `ui.pagination` | defaults to 20 entries |
|  `“All”` | Throws error, expects integer |
|  `-10` | List doesn't have any items in it |
|  `30` | Populates list with 30 entries as default |
|  `10` | Populates list with 10 entries as default|
|  `10` | Populates list with 10 entries as default|
|  `-1`  | Equivalent of All from the drop-down options|

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/srishtyagrawal/storm STORM-2877

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/storm/pull/2505.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2505


commit 3b1bfdbdbe98a141e20f475c8bc08d562a7bda0a
Author: Srishty Agrawal 
Date:   2018-01-03T22:51:25Z

STORM-2877: Add an option to configure pagination in Storm UI




---