Add support for RabbitMQ AMQP 1.0 #46608
Conversation
Signed-off-by: Eddú Meléndez <eddu.melendez@gmail.com>
Signed-off-by: Eddú Meléndez <eddu.melendez@gmail.com>
eddumelendez
force-pushed
the
rabbitmq-amqp
branch
from
77d69d9 to
02ba888
Compare
spring-projects-issues
added
the
status: waiting-for-triage
Signed-off-by: Eddú Meléndez <eddu.melendez@gmail.com>
eddumelendez
changed the title
| @@ -41,6 +41,7 @@ dependencies { | |||
| optional(project(":module:spring-boot-metrics")) | |||
| optional("io.micrometer:micrometer-core") | |||
| optional("org.springframework.amqp:spring-rabbit-stream") | |||
| optional("org.springframework.amqp:spring-rabbitmq-client") | |||
There was a problem hiding this comment.
I think it is better to have an AMQP 1.0 as default one.
Therefore I suggest to do api("org.springframework.amqp:spring-rabbitmq-client") instead of that api("org.springframework.amqp:spring-rabbit").
The spring-rabbitmq-client has spring-rabbit as dependency.
So, both AMQP 1.0 and 0.9.1 are on classpath.
but that should be end-user choice to opt-in to 0.9.1 by excluding spring-rabbitmq-client auto-configuration or so.
I mean this is Spring Boot 4.0 and we probably can bite a bullet and make AMQP 1.0 auto-configured by default.
Just my opinion: we can continue discussion.
There was a problem hiding this comment.
Any comments why this was not addressed?
I think spring-boot-starter-amqp as it is right now, but definitely aimed for AMQP 1.0.
For previous behavior for AMQP 0.9.1, we would need to introduce a new starter - spring-boot-starter-rabbitmq.
There we would exclude this new spring-rabbitmq-client and declare that old spring-rabbit.
But that's my opinion: probably we would need some input from Spring Boot team.
...p/src/main/java/org/springframework/boot/amqp/autoconfigure/RabbitAmqpAutoConfiguration.java
Outdated
Show resolved
Hide resolved
...p/src/main/java/org/springframework/boot/amqp/autoconfigure/RabbitAmqpAutoConfiguration.java
Outdated
Show resolved
Hide resolved
| private RabbitAmqpTemplate rabbitAmqpTemplate; | ||
|
|
||
| public void send(String message) { | ||
| this.rabbitAmqpTemplate.convertAndSend("foo", message); |
There was a problem hiding this comment.
Is foo/bar language OK with Spring Boot team?
| * @author Eddú Meléndez | ||
| * @since 4.0.0 | ||
| */ | ||
| @AutoConfiguration |
There was a problem hiding this comment.
I think need to make before = RabbitAutoConfiguration.class and make that one conditional on something missing from this RabbitAmqpAutoConfiguration.
This is an addition to my previous point: the AMQP 1.0 is by default.
If we don't make them conditional on each other, then both protocols are going to be enabled and connected.
Especially, I worry about RabbitAmqpAdmin and RabbitAdmin which are going to handle topology in parallel but in different connections.
I don't think that we need to give a choice to be able to have both protocols in one Spring Boot application.
|
|
||
| private final RabbitProperties properties; | ||
|
|
||
| RabbitAmqpAutoConfiguration(RabbitProperties properties) { |
There was a problem hiding this comment.
I think we need to look into a separate RabbitAmqpProperties abstraction.
Even if some duplication is possible, a bunch of existing properties are not going to be used for AMQP 1.0 at all.
And might cause confusion.
Signed-off-by: Eddú Meléndez <eddu.melendez@gmail.com>
eddumelendez
force-pushed
the
rabbitmq-amqp
branch
from
4c79b2e to
6572c36
Compare
Signed-off-by: Eddú Meléndez <eddu.melendez@gmail.com>
| * @author Eddú Meléndez | ||
| * @since 4.0.0 | ||
| */ | ||
| @AutoConfiguration(before = RabbitAutoConfiguration.class) |
There was a problem hiding this comment.
Any comments why no @ConditionalOnMissingBean on that RabbitAutoConfiguration?
We should aim to not auto-configure AMQP 0.9.1, if we got AMQP 1.0.
Uh oh!
There was an error while loading. Please reload this page.