Arsnael commented on code in PR #1651:
URL: https://github.com/apache/james-project/pull/1651#discussion_r1305081107
##########
server/apps/distributed-app/src/main/java/org/apache/james/CassandraRabbitMQJamesServerMain.java:
##########
@@ -205,6 +206,21 @@ public static GuiceJamesServer
createServer(CassandraRabbitMQJamesConfiguration
.combineWith(chooseJmapModule(configuration));
}
+ private static Module chooseMailQueue(CassandraRabbitMQJamesConfiguration
configuration) {
+ switch (configuration.getMailQueueChoice()) {
+ case PULSAR:
+ return new PulsarQueueModule();
+ case RABBITMQ:
+ return Modules.combine(
+ new RabbitMQModule(),
+ new RabbitMailQueueRoutesModule(),
+ new RabbitMQMailQueueModule(),
+
MailQueueViewChoice.ModuleChooser.choose(configuration.getMailQueueViewChoice()));
Review Comment:
Ok seriously I think maybe a renaming or some refactoring should be in
order? It seems very rabbitmq specific...
In RabbitMQModule I can see bindings done from conf rabbitmq.properties
file, RabbitMQHealthCheck (???), etc. Are we sure there is no impact? Are those
common for the two mail queues (honestly the health check I quite doubt it for
example)
It's confusing I think for somebody else if he takes a look at the code and
see some rabbitmq naming stuff in a pulsar implementation. I'm not convinced we
should merge it like that.
@chibenwa thoughts on this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
