You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2021/04/22 13:59:21 UTC

[GitHub] [james-project] chibenwa opened a new pull request #400: JAMES-3567 Distributed server should not depend on ActiveMQ

chibenwa opened a new pull request #400:
URL: https://github.com/apache/james-project/pull/400


   Not required, this brings in the JGroup dependency, vulnerable prior version 4.0.
   
   This is a simple dependency cleanup.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] jeantil commented on pull request #400: JAMES-3567 Distributed server should not depend on ActiveMQ

Posted by GitBox <gi...@apache.org>.
jeantil commented on pull request #400:
URL: https://github.com/apache/james-project/pull/400#issuecomment-828601575


   yes, go ahead. sorry for the huge latency these days


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a change in pull request #400: JAMES-3567 Distributed server should not depend on ActiveMQ

Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #400:
URL: https://github.com/apache/james-project/pull/400#discussion_r618868964



##########
File path: server/container/guice/cassandra-rabbitmq-guice/pom.xml
##########
@@ -168,12 +197,48 @@
             <groupId>${james.groupId}</groupId>
             <artifactId>james-server-guice-webadmin</artifactId>
         </dependency>
+        <dependency>
+            <groupId>${james.groupId}</groupId>
+            <artifactId>james-server-guice-webadmin-data</artifactId>

Review comment:
       Again one might compose only some subparts of the webAdmin. An example of this is jpa-smtp that drops all routes related to the mailbox, to JMAP, etc...




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa merged pull request #400: JAMES-3567 Distributed server should not depend on ActiveMQ

Posted by GitBox <gi...@apache.org>.
chibenwa merged pull request #400:
URL: https://github.com/apache/james-project/pull/400


   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on pull request #400: JAMES-3567 Distributed server should not depend on ActiveMQ

Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #400:
URL: https://github.com/apache/james-project/pull/400#issuecomment-828945862


   > yes, go ahead. sorry for the huge latency these days
   
   No problem ;-)


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a change in pull request #400: JAMES-3567 Distributed server should not depend on ActiveMQ

Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #400:
URL: https://github.com/apache/james-project/pull/400#discussion_r618868222



##########
File path: server/container/guice/cassandra-rabbitmq-guice/pom.xml
##########
@@ -156,6 +165,26 @@
             <groupId>${james.groupId}</groupId>
             <artifactId>james-server-guice-jmx</artifactId>
         </dependency>
+        <dependency>

Review comment:
       >Shouldn't these dependencies be in a common guice server definition ? (by these dependencies I mean the ones that are not specific to a particular storage technology.
   
   I could decide to build a server without a given protocol (say JMAP, IMAP, POP3) for instance, to build a pure MTA.
   
   > I had a look and james-server-guice-common feels like a good candidate but for some reason it pulls in inmemory implementations of some components such as james-server-task-memory james-server-mailrepository-memory
   
   Yes some cleanup still to be done, to be honnest.
   
   We might have considered by the past that inMemory dependencies were light with no extra external dependencies thus having them sneaking in was acceptable.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a change in pull request #400: JAMES-3567 Distributed server should not depend on ActiveMQ

Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #400:
URL: https://github.com/apache/james-project/pull/400#discussion_r618866456



##########
File path: server/container/guice/cassandra-rabbitmq-guice/pom.xml
##########
@@ -110,10 +115,6 @@
             <artifactId>event-sourcing-event-store-memory</artifactId>
             <scope>test</scope>
         </dependency>
-        <dependency>
-            <groupId>${james.groupId}</groupId>

Review comment:
       Yes. A guice server bundling Cassandra, ES, ActiveMQ.
   
   We suffer from having no clearer separation between our guice modules and our apps.
   
   I would be in favor of something like this:
   
   ```
   server/apps/spring
   server/apps/memory
   server/apps/distributed
   server/apps/jpa
   server/apps/jpa-smtp
   server/apps/cassandra //-es-activemq
   server/containers/guice/... // all module definitions would go here
   ```




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] jeantil commented on a change in pull request #400: JAMES-3567 Distributed server should not depend on ActiveMQ

Posted by GitBox <gi...@apache.org>.
jeantil commented on a change in pull request #400:
URL: https://github.com/apache/james-project/pull/400#discussion_r618750516



##########
File path: server/container/guice/cassandra-rabbitmq-guice/pom.xml
##########
@@ -156,6 +165,26 @@
             <groupId>${james.groupId}</groupId>
             <artifactId>james-server-guice-jmx</artifactId>
         </dependency>
+        <dependency>

Review comment:
       I feel there must be something wrong in the way we assemble servers. 
   
   Shouldn't these dependencies be in a common guice server definition ? (by these dependencies I mean the ones that are not specific to a particular storage technology. 
   
   I had a look and `james-server-guice-common` feels like a good candidate but for some reason it pulls in inmemory implementations of some components  such as `james-server-task-memory` `james-server-mailrepository-memory`

##########
File path: server/container/guice/cassandra-rabbitmq-guice/pom.xml
##########
@@ -110,10 +115,6 @@
             <artifactId>event-sourcing-event-store-memory</artifactId>
             <scope>test</scope>
         </dependency>
-        <dependency>
-            <groupId>${james.groupId}</groupId>

Review comment:
       If I understand correctly `jame-server-cassandra-guice` is in fact `jame-server-cassandra-activemq-guice` ?

##########
File path: server/container/guice/cassandra-rabbitmq-guice/pom.xml
##########
@@ -168,12 +197,48 @@
             <groupId>${james.groupId}</groupId>
             <artifactId>james-server-guice-webadmin</artifactId>
         </dependency>
+        <dependency>
+            <groupId>${james.groupId}</groupId>
+            <artifactId>james-server-guice-webadmin-data</artifactId>

Review comment:
       james-server-guice-webmin-common ? I'm not saying to change it for this PR but I think it would be interesting.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on pull request #400: JAMES-3567 Distributed server should not depend on ActiveMQ

Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #400:
URL: https://github.com/apache/james-project/pull/400#issuecomment-828555377


   @jeantil would you agree with this work "as is"?


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org