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 2022/08/03 09:53:39 UTC

[GitHub] [james-project] vttranlina opened a new pull request, #1104: [Draft] Guice support the custom task in extension

vttranlina opened a new pull request, #1104:
URL: https://github.com/apache/james-project/pull/1104

   ## Why
   - James is supporting the custom web admin (https://james.apache.org/howTo/custom-webadmin-routes.html), but It does not yet support the `task-json` in the extension. 
   
   We got an error when trying to guice bind more `AdditionalInformationDTOModule` to Set in the extension module. 
   Eg:
   
   ```java
       @Named(DTOModuleInjections.WEBADMIN_DTO)
       @ProvidesIntoSet
       public AdditionalInformationDTOModule<? extends TaskExecutionDetails.AdditionalInformation, ? extends AdditionalInformationDTO> webAdminFeedHamAdditionalInformation() {
           return FeedHamToRSpamDTaskAdditionalInformationDTO.SERIALIZATION_MODULE;
       }
   ```
   
   -> It will be better if have a mechanism for support that 
   
   ## How 
   - Add one more configure for `webadmin-dto` guice in `webadmin.properties`. Eg: `extensions.dtos=org.apache.james.modules.server.WebAdminDTOExtensionModuleImpl` 
   - Add one more configure for `task-json` guice in `extensions.properties` 
   Eg: `guice.extension.task=org.apache.james.server.task.json.TaskExtensionModuleImpl` 
   
   


-- 
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: notifications-unsubscribe@james.apache.org

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] Arsnael commented on pull request #1104: JAMES-3796 Guice support the custom task in extension

Posted by GitBox <gi...@apache.org>.
Arsnael commented on PR #1104:
URL: https://github.com/apache/james-project/pull/1104#issuecomment-1207580166

   You tag the first commit as no review, while I'm pretty sure it's part of this PR though?


-- 
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: notifications-unsubscribe@james.apache.org

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 diff in pull request #1104: [Draft] Guice support the custom task in extension

Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1104:
URL: https://github.com/apache/james-project/pull/1104#discussion_r937502046


##########
server/container/guice/protocols/webadmin/src/main/java/org/apache/james/modules/server/TaskRoutesModule.java:
##########
@@ -48,9 +49,11 @@ protected void configure() {
     @Named(DTOModuleInjections.WEBADMIN_DTO)
     @Provides
     @Singleton
-    public DTOConverter<TaskExecutionDetails.AdditionalInformation, AdditionalInformationDTO> additionalInformationDTOConverter(
-            @Named(DTOModuleInjections.WEBADMIN_DTO) Set<AdditionalInformationDTOModule<? extends TaskExecutionDetails.AdditionalInformation, ? extends AdditionalInformationDTO>> modules) {
-
-        return new DTOConverter<>(modules);
+    public DTOConverter<TaskExecutionDetails.AdditionalInformation, AdditionalInformationDTO> additionalInformationDTOConverter(@Named(DTOModuleInjections.WEBADMIN_DTO) Set<AdditionalInformationDTOModule<? extends TaskExecutionDetails.AdditionalInformation, ? extends AdditionalInformationDTO>> modules,
+                                                                                                                                @Named(DTOModuleInjections.CUSTOM_WEBADMIN_DTO) Set<AdditionalInformationDTOModule<? extends TaskExecutionDetails.AdditionalInformation, ? extends AdditionalInformationDTO>> customModules) {

Review Comment:
   ```suggestion                                                                                                                           @Named(TaskModuleInjectionKeys.ADDITIONAL_INFORMATION_DTO) Set<AdditionalInformationDTOModule<? extends TaskExecutionDetails.AdditionalInformation, ? extends AdditionalInformationDTO>> customModules) {
   ```
   
   ?



-- 
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: notifications-unsubscribe@james.apache.org

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 #1104: [Draft] Guice support the custom task in extension

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

   Having separate DTOs for task/webadmin is mostly due to historic reason (different formats + try to prevent breaking change).
   
   For newly developped extentions tasks we could force webadmin-dto==task-dto which would:
   
    - Avoid confusion for our users
    - Simplify the conf and allow to keep it only in extensions.properties <3
    - Produce less code.


-- 
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: notifications-unsubscribe@james.apache.org

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] Arsnael commented on pull request #1104: JAMES-3796 Guice support the custom task in extension

Posted by GitBox <gi...@apache.org>.
Arsnael commented on PR #1104:
URL: https://github.com/apache/james-project/pull/1104#issuecomment-1207594428

   My bad, morning :)


-- 
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: notifications-unsubscribe@james.apache.org

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] vttranlina commented on a diff in pull request #1104: JAMES-3796 Guice support the custom task in extension

Posted by GitBox <gi...@apache.org>.
vttranlina commented on code in PR #1104:
URL: https://github.com/apache/james-project/pull/1104#discussion_r938636006


##########
server/apps/distributed-app/docs/modules/ROOT/pages/configure/extensions.adoc:
##########
@@ -25,4 +25,34 @@ guice.extension.module=com.project.MyServiceModule
 
 Enables to inject MyService into your extensions.
 
+
+*guice.extension.module*: come separated list of fully qualified class name. These classes need to implement Task extension modules.
+
+Here is an example of such a class :
+
+....
+public class RSpamDTaskExtensionModule implements TaskExtensionModule {
+
+    @Inject
+    public RSpamDTaskExtensionModule() {
+    }

Review Comment:
   just a sample of the document
   
   the class should have Inject annotation with constructor for guice 



-- 
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: notifications-unsubscribe@james.apache.org

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] vttranlina commented on pull request #1104: JAMES-3796 Guice support the custom task in extension

Posted by GitBox <gi...@apache.org>.
vttranlina commented on PR #1104:
URL: https://github.com/apache/james-project/pull/1104#issuecomment-1207583509

   > You tag the first commit as no review, while I'm pretty sure it's part of this PR though?
   
   I don't see any no_review tag.
   ![image](https://user-images.githubusercontent.com/81145350/183326097-4af5fd13-e349-4b3d-a54f-0483fc3b1071.png)
   


-- 
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: notifications-unsubscribe@james.apache.org

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] quantranhong1999 commented on a diff in pull request #1104: JAMES-3796 Guice support the custom task in extension

Posted by GitBox <gi...@apache.org>.
quantranhong1999 commented on code in PR #1104:
URL: https://github.com/apache/james-project/pull/1104#discussion_r938619880


##########
server/apps/distributed-app/docs/modules/ROOT/pages/configure/extensions.adoc:
##########
@@ -25,4 +25,34 @@ guice.extension.module=com.project.MyServiceModule
 
 Enables to inject MyService into your extensions.
 
+
+*guice.extension.module*: come separated list of fully qualified class name. These classes need to implement Task extension modules.
+
+Here is an example of such a class :
+
+....
+public class RSpamDTaskExtensionModule implements TaskExtensionModule {
+
+    @Inject
+    public RSpamDTaskExtensionModule() {
+    }

Review Comment:
   Why do we need 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: notifications-unsubscribe@james.apache.org

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 diff in pull request #1104: JAMES-3796 Guice support the custom task in extension

Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1104:
URL: https://github.com/apache/james-project/pull/1104#discussion_r939790593


##########
server/apps/distributed-app/docs/modules/ROOT/pages/configure/extensions.adoc:
##########
@@ -25,4 +25,34 @@ guice.extension.module=com.project.MyServiceModule
 
 Enables to inject MyService into your extensions.
 
+
+*guice.extension.module*: come separated list of fully qualified class name. These classes need to implement Task extension modules.

Review Comment:
   ` These classes need to implement Task extension modules.`
   
   Please do this in another paragraph:
   ```
   *guice.extension.module*: come separated list of fully qualified class name.
   
   Eg: 
   
   
   
   Etensions can rely on the Task manager to supervise long running task execution (progress, await, cancelation, scheduling...). These extensions need to implement Task extension modules. Here is an example of such a class :



-- 
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: notifications-unsubscribe@james.apache.org

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 #1104: JAMES-3796 Guice support the custom task in extension

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


-- 
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: notifications-unsubscribe@james.apache.org

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