[GitHub] activemq-artemis pull request #1773: ARTEMIS-1589 ActiveMQProtonRemotingConn...

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

[GitHub] activemq-artemis pull request #1773: ARTEMIS-1589 ActiveMQProtonRemotingConn...

RaiSaurabh
GitHub user jostbg opened a pull request:

    https://github.com/apache/activemq-artemis/pull/1773

    ARTEMIS-1589 ActiveMQProtonRemotingConnection#getClientID() now returns remote client Id...

    ...instead of server client Id

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

    $ git pull https://github.com/jostbg/activemq-artemis ARTEMIS-1589

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

    https://github.com/apache/activemq-artemis/pull/1773.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 #1773
   
----
commit 47dd6f19c0ead5f01a526ecd941f2a32dace8f0d
Author: jostbg <35264802+jostbg@...>
Date:   2018-01-11T11:23:33Z

    ARTEMIS-1589 ActiveMQProtonRemotingConnection#getClientID() now returns remote client Id instead of server client Id

----


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

[GitHub] activemq-artemis issue #1773: ARTEMIS-1589 ActiveMQProtonRemotingConnection#...

RaiSaurabh
Github user jostbg commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1773
 
    @jbertram Could you please check if my [observations](https://issues.apache.org/jira/browse/ARTEMIS-1589) and my trivial fix are correct. Thanks in advance!


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

[GitHub] activemq-artemis issue #1773: ARTEMIS-1589 ActiveMQProtonRemotingConnection#...

RaiSaurabh
In reply to this post by RaiSaurabh
Github user jbertram commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1773
 
    @jostbg, I'm not terribly familiar with AMQP so I can't comment with authority as to whether or not this is a good change.  @tabish121, could you perhaps take a look here?


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

[GitHub] activemq-artemis issue #1773: ARTEMIS-1589 ActiveMQProtonRemotingConnection#...

RaiSaurabh
In reply to this post by RaiSaurabh
Github user tabish121 commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1773
 
    @jbertram @jostbg As far as I can see the patch looks correct, it would seem to make more sense to return the remote container ID here than the locally generated server side value as that provides more context into the remote connection.  That is of course if the getClientID API in RemotingConnection is referring to the value specified by the remote, which would make the most sense.  


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

[GitHub] activemq-artemis issue #1773: ARTEMIS-1589 ActiveMQProtonRemotingConnection#...

RaiSaurabh
In reply to this post by RaiSaurabh
Github user jostbg commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1773
 
    The java doc of getClientID says "Returns the Client ID associated with this connection" which probably refers to the "client-side" ID. StompConnection for example returns the ID set by the client and not some server generated ID.


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

[GitHub] activemq-artemis pull request #1773: ARTEMIS-1589 ActiveMQProtonRemotingConn...

RaiSaurabh
In reply to this post by RaiSaurabh
Github user asfgit closed the pull request at:

    https://github.com/apache/activemq-artemis/pull/1773


---