[GitHub] activemq-artemis pull request #1628: ARTEMIS-1364: Enable internal sorting i...

Next Topic
 
classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1628: ARTEMIS-1364: Enable internal sorting i...

michaelandrepearce-2
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 <[hidden email]>
Date:   2017-11-01T08:37:46Z

    ARTEMIS-1364: Enable internal sorting in hawtio web console

----


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...

michaelandrepearce-2
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.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
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?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
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.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
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?  


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
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.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
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.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user mtaylor commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1628
 
    @RaiSaurabh Awesome.  Thanks.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
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.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
Github user RaiSaurabh commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1628
 
    Will fix and commit the indentation issue.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1628: ARTEMIS-1364: Enable internal sorting i...

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
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.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1628: ARTEMIS-1364: Enable internal sorting i...

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
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.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
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.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
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.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1628: ARTEMIS-1364: Enable internal sorting in Hawti...

michaelandrepearce-2
In reply to this post by michaelandrepearce-2
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.


---