Arsnael commented on code in PR #3194:
URL: https://github.com/apache/james-project/pull/3194#discussion_r4140292736


##########
server/container/guice/queue/activemq/src/main/java/org/apache/james/modules/queue/activemq/ActiveMQQueueModule.java:
##########
@@ -50,33 +48,40 @@
 import com.google.inject.multibindings.Multibinder;
 import com.google.inject.multibindings.ProvidesIntoSet;
 
+/**
+ * Guice module for the Artemis-backed mail queue.
+ * Replaces the legacy ActiveMQ embedded broker with Apache ActiveMQ Artemis.
+ */
 public class ActiveMQQueueModule extends AbstractModule {
 
     private static final Logger LOGGER = 
LoggerFactory.getLogger(ActiveMQQueueModule.class);
     private static final String FILENAME = "activemq";
 
     @Override
     protected void configure() {
-        bind(PersistenceAdapter.class).to(KahaDBPersistenceAdapter.class);
-        bind(KahaDBPersistenceAdapter.class).in(Scopes.SINGLETON);
         bind(EmbeddedActiveMQ.class).in(Scopes.SINGLETON);
         bind(ActiveMQMailQueueFactory.class).in(Scopes.SINGLETON);
-        
bind(ActiveMQMetricCollector.class).to(ActiveMQMetricCollectorImpl.class);
-        bind(ActiveMQMetricCollectorImpl.class).in(Scopes.SINGLETON);
+        // Use Noop metric collector since Artemis does not support ActiveMQ 
Statistics Plugin.
+        // Artemis metrics can be obtained via JMX or dedicated management 
APIs.
+        
bind(ActiveMQMetricCollector.class).to(ActiveMQMetricCollectorNoop.class);
+        bind(ActiveMQMetricCollectorNoop.class).in(Scopes.SINGLETON);

Review Comment:
   I guess we consider that metrics we go directly getting them on Artemis and 
not from James anymore right? I'm not against it, but is everybody ok with that 
decision?



##########
server/container/spring/pom.xml:
##########
@@ -128,15 +128,10 @@
             <scope>provided</scope>
         </dependency>
         <dependency>
-            <groupId>org.apache.activemq</groupId>
-            <artifactId>activemq-spring</artifactId>
-            <version>${activemq.version}</version>
-            <exclusions>
-                <exclusion>
-                    <groupId>commons-logging</groupId>
-                    <artifactId>commons-logging</artifactId>
-                </exclusion>
-            </exclusions>
+            <!-- Artemis Jakarta Server replaces legacy activemq-spring for 
embedded broker -->
+            <groupId>org.apache.artemis</groupId>
+            <artifactId>artemis-jakarta-server</artifactId>
+            <version>${activmq-artemis.version}</version>

Review Comment:
   shouldnt spring app get an activemq.properties configuration in its 
sample-configuration folder as well then?



-- 
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]

Reply via email to