You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by RaiSaurabh <gi...@git.apache.org> on 2017/11/01 08:41:20 UTC
[GitHub] activemq-artemis pull request #1628: ARTEMIS-1364: Enable internal sorting i...
GitHub user RaiSaurabh opened a pull request:
https://github.com/apache/activemq-artemis/pull/1628
ARTEMIS-1364: Enable internal sorting in Hawtio web console
Enabled default sorting on the Hawtio web console.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/RaiSaurabh/activemq-artemis sarai
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/1628.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 #1628
----
commit c657b20970886ea3206cb1eefd37b45b1622bc23
Author: saurabhrai <ra...@hotmail.com>
Date: 2017-11-01T08:37:46Z
ARTEMIS-1364: Enable internal sorting in hawtio web console
----
---
[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...
Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on the issue:
https://github.com/apache/activemq-artemis/pull/1628
Nice work @RaiSaurabh. Thanks a lot
---
[GitHub] activemq-artemis pull request #1628: ARTEMIS-1364: Enable internal sorting i...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1628
---
[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...
Posted by RaiSaurabh <gi...@git.apache.org>.
Github user RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1628
@mtaylor @michaelandrepearce Could you please merge the PR.
---
[GitHub] activemq-artemis pull request #1628: ARTEMIS-1364: Enable internal sorting i...
Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1628#discussion_r151136266
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerConsumerImpl.java ---
@@ -1387,4 +1387,49 @@ public boolean isBrowsed() {
return b;
}
}
+
+ @Override
+ public long getSequentialID() {
+ return sequentialID;
+ }
+
+ @Override
+ public SimpleString getQueueName() {
+ return getQueue().getName();
+ }
+
+ @Override
+ public RoutingType getQueueType() {
+ return getQueue().getRoutingType();
+ }
+
+ @Override
+ public SimpleString getQueueAddress() {
+ return getQueue().getAddress();
+ }
+
+ @Override
+ public String getSessionName() {
+ return server.getSessionByID(getSessionID()).getName();
+ }
+
+ @Override
+ public String getConnectionClientID() {
+ return server.getSessionByID(getSessionID()).getRemotingConnection().getClientID();
+ }
--- End diff --
Same comment as above and in all other places you are doing the lookup.
---
[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1628
@mtaylor i think we were looking at pushing sort down to broker as will be more elements than in FE table when many queues/address etc. Could you confirm?
---
[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...
Posted by RaiSaurabh <gi...@git.apache.org>.
Github user RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1628
@mtaylor I have updated the code and squashed it into one commit. The build was successful.
---
[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...
Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on the issue:
https://github.com/apache/activemq-artemis/pull/1628
@RaiSaurabh Awesome. Thanks.
---
[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...
Posted by RaiSaurabh <gi...@git.apache.org>.
Github user RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1628
Will fix and commit the indentation issue.
---
[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...
Posted by RaiSaurabh <gi...@git.apache.org>.
Github user RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1628
Got the CheckStyle validations. Will fix them and then commit and then squash.
---
[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...
Posted by RaiSaurabh <gi...@git.apache.org>.
Github user RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1628
@mtaylor @michaelandrepearce The current implementation we set the sort column and option in the filter and pass it along to the Artemis server for the operation. But looking at the code on the Artemis server I see that it only check for the Filter components and not sorting. No such functionality is written for sorting of columns. With the default sorting operation provided by the Angular grid, I am able to sort the response table even on paging. Should not we go ahead with this?
---
[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...
Posted by RaiSaurabh <gi...@git.apache.org>.
Github user RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1628
For sorting purpose AngularJS grid internal sorting should be enough for sorting columns. I checked it was not sorting on other tabs as well (Connections, Sessions, Consumers, Producers, Addresses and Queues). I tested the UI after the change and sorting works.
---
[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...
Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on the issue:
https://github.com/apache/activemq-artemis/pull/1628
@RaiSaurabh @michaelandrepearce Since the results are paged, we need to sort on the broker side. If it doesn't work already then that's a bug on the broker that needs addressing.
---
[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...
Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on the issue:
https://github.com/apache/activemq-artemis/pull/1628
@RaiSaurabh I am not sure how it can work with paging. Since the full list needs to be sorted before the pages are populated. This solution will receive the first N (page size) unsorted elements from a collection, then sort them, meaning only the page results are sorted. We should instead implement this on the broker so that sorting + paging works.
---
[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...
Posted by RaiSaurabh <gi...@git.apache.org>.
Github user RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1628
@mtaylor Ok. Thanks, I will start working on doing it in broker end. Will update once done.
---
[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...
Posted by RaiSaurabh <gi...@git.apache.org>.
Github user RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1628
Added the code to support sorting on the server side. Request @mtaylor to please review.
---
[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...
Posted by RaiSaurabh <gi...@git.apache.org>.
Github user RaiSaurabh commented on the issue:
https://github.com/apache/activemq-artemis/pull/1628
@mtaylor This is strange the Checkstyle is not failing on my end but it is on the Jenkins.
---
[GitHub] activemq-artemis pull request #1628: ARTEMIS-1364: Enable internal sorting i...
Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1628#discussion_r151136139
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerConsumerImpl.java ---
@@ -1387,4 +1387,49 @@ public boolean isBrowsed() {
return b;
}
}
+
+ @Override
+ public long getSequentialID() {
+ return sequentialID;
+ }
+
+ @Override
+ public SimpleString getQueueName() {
+ return getQueue().getName();
+ }
+
+ @Override
+ public RoutingType getQueueType() {
+ return getQueue().getRoutingType();
+ }
+
+ @Override
+ public SimpleString getQueueAddress() {
+ return getQueue().getAddress();
+ }
+
+ @Override
+ public String getSessionName() {
+ return server.getSessionByID(getSessionID()).getName();
+ }
--- End diff --
You already have access to the session in the ServerConsumerImpl.
---