Tradeshift / spring-rabbitmq-tuning

This library makes it easy to configure RabbitMQ for use with Spring
MIT License
43 stars 26 forks source link

Connection to a RabbitMQ cluster #35

Closed rrnovaesjr closed 4 years ago

rrnovaesjr commented 4 years ago

Hi everyone,

I was investigating the properties on https://github.com/Tradeshift/spring-rabbitmq-tuning/wiki/Properties-Documentation and did not find anything that was supposed to be similar to Spring's spring.rabbitmq.addresses property.

Is this library compatible with Spring's properties as well? Is there any way to connect to a cluster?

wesleyegberto commented 4 years ago

Hi, I'm trying some changes to support that.

This lib is using Spring Amqp but not its properties (like spring.rabbitmq.addresses).

Currently it cannot connect to a cluster because of how it is creating a ConnectionFactory, by only calling:

factory.setHost(property.getHost());
factory.setPort(property.getPort());

I think the lib will need to change this a bit so Spring class could use the method ConnectionFactory.createConnection(addressesList) (CachingConnectionFactory will call that method when it has the property addresses filled).

Maybe this new properties could help define which way to go.

spring.rabbitmq.custom.<YOUR_EVENT>.clusterMode=true
spring.rabbitmq.custom.<YOUR_EVENT>.hosts=localhost:5672,localhost:6672,localhost:7672
if (property.isClusterMode()) {
        CachingConnectionFactory connFact = new CachingConnectionFactory(factory);
        connFact.setAddresses(property.getHosts());
        return cachingConnectionFactory;
}
factory.setHost(property.getHost());
factory.setPort(property.getPort());
return new CachingConnectionFactory(factory);

@juliofalbo , what do you think?

juliofalbo commented 4 years ago

First I would like @rrnovaesjr for the issue!

Now, talking about the response, I fully agree with you @wesleyegberto !

It looks a little change that will give to us a great feature.

Will you drive this issue @wesleyegberto ?

Would be amazing!!!

wesleyegberto commented 4 years ago

Sure.

Here is the commit to enable connection to cluster that I've been using.

I need some review in this points:

If this naming strategy is ok I'm planning to change any ref to the methods from RabbitBeanNameResolver that receive host and port to force the use of properties object.

After that I will write the tests.

wesleyegberto commented 4 years ago

As the PR #38 was merged I think this can be closed.

rrnovaesjr commented 4 years ago

Hi everyone! Thanks a lot for all the help. Appreciate it!