You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by lulf <gi...@git.apache.org> on 2016/10/21 14:12:11 UTC

[GitHub] activemq-artemis pull request #855: Support outgoing AMQP connections

GitHub user lulf opened a pull request:

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

    Support outgoing AMQP connections

    @mtaylor This is the initial version. The first commit is something that I was surprised that was not there, so let me know if it's bad. Without it, I have to call connectionCreated explicitly.
    
    The second commit fixes an issue I saw when testing, where it looked like onFlow was called on a session that had already been closed.
    
    The third commit is the actual implementation of outgoing connections. The code was originally a part of the AMQP connector service that is left out of this commit, but I thought it was a better fit inside the AMQP implementation. In the integration test you can see that its a bit awkward in its use, so I think it would benefit from a refactoring at some point.

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

    $ git pull https://github.com/EnMasseProject/activemq-artemis ARTEMIS-814

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

    https://github.com/apache/activemq-artemis/pull/855.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 #855
    
----
commit be5aebfac527b3744742ff63cc348276a9b38a66
Author: Ulf Lilleengen <lu...@redhat.com>
Date:   2016-10-21T13:14:51Z

    ARTEMIS-814: Notify connection listener of connection created

commit 9e13d8702fd434033747e9af8c1108f12e35349e
Author: Ulf Lilleengen <lu...@redhat.com>
Date:   2016-10-21T13:22:46Z

    ARTEMIS-814: Fix a bug where context could be null in case a connection was closed before the flow arrived

commit 00b64342c15828dbd96153d7070f4f01dc61d3cd
Author: Ulf Lilleengen <lu...@redhat.com>
Date:   2016-10-21T13:24:16Z

    ARTEMIS-814: Add support for outgoing AMQP connections

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis issue #855: Support outgoing AMQP connections

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/855
  
    @lulf  ok.. will merge it then


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request #855: Support outgoing AMQP connections

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis issue #855: Support outgoing AMQP connections

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/855
  
    @mtaylor @lulf , shouldn't we squash them all into a single commit?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis issue #855: Support outgoing AMQP connections

Posted by lulf <gi...@git.apache.org>.
Github user lulf commented on the issue:

    https://github.com/apache/activemq-artemis/pull/855
  
    @clebertsuconic In EnMasse(messaging-as-a-service), we use artemis for addresses that require store-and-forward semantics. Today, in order to have the broker connected to the router network, we co-locate a dispatch router together with the artemis instance. This limits our scalability and adds an extra hop for messages. We want to have the broker connect to the router network instead using a connector service, so that we can get rid of the co-located router.
    
    This PR contains the changes I had to make in artemis for the connector service to work. It currently lacks the capability to establish sessions and outgoing links, which we might need later on. That being said, the only changes strictly required is the open() method on the ProtonHandler and AMQPConnectionContext. I can leave the  ProtonClientConnectionLifeCycleListener and ProtonClientProtocolManager inside the connector service instead if that is desired.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis issue #855: Support outgoing AMQP connections

Posted by lulf <gi...@git.apache.org>.
Github user lulf commented on the issue:

    https://github.com/apache/activemq-artemis/pull/855
  
    @clebertsuconic Added, thanks.
    
    I actually started out trying both the current 'integrated' approach and using vertx-proton. I decided to go down the current path based after initial feedback from @andytaylor and @mtaylor on the 'integrated' approach. Using the artemis implementation gives me the connection management and integration with queues/topics, which I think would require more work if going through a client library path. 
    
    That being said, more work is probably required if/when we need to establish outgoing amqp links.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis issue #855: Support outgoing AMQP connections

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/855
  
    @lulf ok, I would merge it with this commit also:
    
    https://github.com/clebertsuconic/activemq-artemis/commit/efc6e2dccf8ba8ae3e3909ba26ff8c4122bb8d0e
    
    
    Did you consider using the Client API from qpid-proton-jms? or open a JMS connection from qpid-proton?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis issue #855: Support outgoing AMQP connections

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/855
  
    @lulf the first two commits are definitely a keeper... the third one I'm not so sure.
    
    I had in the past tried to implement core over AMQP ,which is something I gave up given the qpid-proton-jms being much more mature. I don't really want to invest all the time @tabish121  already invested (think about a good 1 year and half to 2 years) to make his client be matured enough to support production scenarios.
    
    Just a FYI, I had recently removed a lot of code that was supporting client AMQP over the Proton client to simplify things around the server. so I'm really thinking this is not the right place to go.
    
    
    if you want and have the stamina (think of a multi year commitment) to support core over AMQP (which is the ClientProtocolManager) then we could consider it as an option, otherwise I am really thinking this is not the right way to go.
    
    
    @lulf  / @mtaylor what is the use case here? maybe I'm mistaken and missing something here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---