quantranhong1999 commented on code in PR #1651:
URL: https://github.com/apache/james-project/pull/1651#discussion_r1298244499


##########
server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPConfiguration.java:
##########
@@ -114,11 +115,16 @@ public Builder maximumSendSize(Optional<Long> 
maximumSendSize) {
             return this;
         }
 
+        public Builder supportsDelaySends(Optional<Boolean> 
supportsDelaySends) {
+            this.supportsDelaySends = supportsDelaySends;
+            return this;
+        }
+
         public JMAPConfiguration build() {
             Preconditions.checkState(enabled.isPresent(), "You should specify 
if JMAP server should be started");
             return new JMAPConfiguration(enabled.get(), port, 
emailQueryViewEnabled.orElse(false),
                 userProvisioningEnabled.orElse(true),
-                defaultVersion.orElse(Version.DRAFT), maximumSendSize);
+                defaultVersion.orElse(Version.DRAFT), maximumSendSize, 
supportsDelaySends.get());

Review Comment:
   Never use optional.get directly. If it is an `Optional.empty`, you would get 
a `NullPointerException`.
   
   Suggestion: use `.orElse(defaultValue)`.



##########
server/apps/distributed-app/docker-compose-with-pulsar.yml:
##########
@@ -0,0 +1,83 @@
+version: '3'
+
+services:
+
+  james:
+    image: apache/james:distributed-latest
+    container_name: james
+    hostname: james.local
+    command:
+      - --generate-keystore
+    networks:
+      - james
+    ports:
+      - "80:80"
+      - "25:25"
+      - "110:110"
+      - "143:143"
+      - "465:465"
+      - "587:587"
+      - "993:993"
+      - "8000:8000"
+    volumes:
+      - ./sample-configuration/pulsar.properties:/root/conf/pulsar.properties
+      - ./sample-configuration/keystore:/root/conf/keystore

Review Comment:
   BTW if you already use `--generate-keystore`, IMO you should not need to 
volume keystore.



##########
server/apps/distributed-app/docs/modules/ROOT/pages/configure/queue.adoc:
##########
@@ -0,0 +1,14 @@
+= Distributed James Server &mdash; queue.properties
+:navtitle: queue.properties
+
+This configuration helps you configure mail queue you want to select.

Review Comment:
   "which Mail Queue technology"



##########
server/container/guice/queue/rabbitmq/src/main/java/org/apache/james/modules/queue/rabbitmq/RabbitMQModule.java:
##########
@@ -71,31 +65,6 @@ protected void configure() {
         
healthCheckMultiBinder.addBinding().to(RabbitMQMailQueueDeadLetterQueueHealthCheck.class);

Review Comment:
   IMO `RabbitMQMailQueueDeadLetterQueueHealthCheck` guice binding should be in 
`RabbitMQMailQueueModule` instead.



##########
server/apps/distributed-app/docs/modules/ROOT/pages/configure/queue.adoc:
##########
@@ -0,0 +1,14 @@
+= Distributed James Server &mdash; queue.properties
+:navtitle: queue.properties
+
+This configuration helps you configure mail queue you want to select.
+
+== RabbitMQ Configuration
+
+.rabbitmq.properties content

Review Comment:
   Why there is a RabbitMQ conf in this file?



##########
server/apps/distributed-app/docs/modules/ROOT/pages/configure/queue.adoc:
##########
@@ -0,0 +1,14 @@
+= Distributed James Server &mdash; queue.properties
+:navtitle: queue.properties
+
+This configuration helps you configure mail queue you want to select.
+
+== RabbitMQ Configuration
+
+.rabbitmq.properties content
+|===
+| Property name | explanation
+
+| queue.choice
+| Mail queue can be implemented by many technologies, Pulsar, RabbitMQ,.... 
This property will choose which mail queue you want

Review Comment:
   Name exactly which mail queue options are available (RABBITMQ / PULSAR), and 
which is the default choice.



##########
server/apps/distributed-app/docker-compose-with-pulsar.yml:
##########
@@ -0,0 +1,83 @@
+version: '3'
+
+services:
+
+  james:
+    image: apache/james:distributed-latest
+    container_name: james
+    hostname: james.local
+    command:
+      - --generate-keystore
+    networks:
+      - james
+    ports:
+      - "80:80"
+      - "25:25"
+      - "110:110"
+      - "143:143"
+      - "465:465"
+      - "587:587"
+      - "993:993"
+      - "8000:8000"
+    volumes:
+      - ./sample-configuration/pulsar.properties:/root/conf/pulsar.properties
+      - ./sample-configuration/keystore:/root/conf/keystore

Review Comment:
   You do not override the default `queue.properties` (with RabbitMQ) by a 
Pulsar mail queue choice?



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