You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2022/06/03 03:15:45 UTC

[GitHub] [dolphinscheduler] kezhenxu94 commented on a diff in pull request #10358: Remove quartz at WorkerServer

kezhenxu94 commented on code in PR #10358:
URL: https://github.com/apache/dolphinscheduler/pull/10358#discussion_r888576336


##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/WorkerServer.java:
##########
@@ -54,11 +54,22 @@
 import org.springframework.boot.SpringApplication;
 import org.springframework.boot.autoconfigure.SpringBootApplication;
 import org.springframework.context.annotation.ComponentScan;
+import org.springframework.context.annotation.FilterType;
 import org.springframework.transaction.annotation.EnableTransactionManagement;
 
 @SpringBootApplication
 @EnableTransactionManagement
-@ComponentScan("org.apache.dolphinscheduler")
+@ComponentScan(basePackages = "org.apache.dolphinscheduler",
+        excludeFilters = {
+                @ComponentScan.Filter(type = FilterType.REGEX, pattern = {
+                        "org.apache.dolphinscheduler.service.k8s.*",
+                        "org.apache.dolphinscheduler.service.permission.*",

Review Comment:
   I think `o.a.d.service.permission` should be moved to `api` module, it's only used there, and should only be used there?



##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/WorkerServer.java:
##########
@@ -102,6 +113,8 @@ public class WorkerServer implements IStoppable {
     @Autowired
     private WorkerRegistryClient workerRegistryClient;
 
+    // todo: Can we just load the task spi, and don't install into mysql?
+    //  we don't need to rely the dao module in worker.

Review Comment:
   Might be possible in https://github.com/apache/dolphinscheduler/pull/9296



##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/WorkerServer.java:
##########
@@ -54,11 +54,22 @@
 import org.springframework.boot.SpringApplication;
 import org.springframework.boot.autoconfigure.SpringBootApplication;
 import org.springframework.context.annotation.ComponentScan;
+import org.springframework.context.annotation.FilterType;
 import org.springframework.transaction.annotation.EnableTransactionManagement;
 
 @SpringBootApplication
 @EnableTransactionManagement
-@ComponentScan("org.apache.dolphinscheduler")
+@ComponentScan(basePackages = "org.apache.dolphinscheduler",
+        excludeFilters = {
+                @ComponentScan.Filter(type = FilterType.REGEX, pattern = {
+                        "org.apache.dolphinscheduler.service.k8s.*",

Review Comment:
   The same, should be moved to api module



##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/WorkerServer.java:
##########
@@ -54,11 +54,22 @@
 import org.springframework.boot.SpringApplication;
 import org.springframework.boot.autoconfigure.SpringBootApplication;
 import org.springframework.context.annotation.ComponentScan;
+import org.springframework.context.annotation.FilterType;
 import org.springframework.transaction.annotation.EnableTransactionManagement;
 
 @SpringBootApplication
 @EnableTransactionManagement
-@ComponentScan("org.apache.dolphinscheduler")
+@ComponentScan(basePackages = "org.apache.dolphinscheduler",
+        excludeFilters = {
+                @ComponentScan.Filter(type = FilterType.REGEX, pattern = {
+                        "org.apache.dolphinscheduler.service.k8s.*",
+                        "org.apache.dolphinscheduler.service.permission.*",
+                        "org.apache.dolphinscheduler.service.process.*",
+                        "org.apache.dolphinscheduler.service.quartz.*",
+                        "org.apache.dolphinscheduler.service.queue.*",

Review Comment:
   I'm big +1 👍  on your Comments in the issue "move quartz" to a separate module and make it a plugin. Also, we should consider removing `dolphinscheduler-seryice` one day because that name can lead to misuse, people put all `***Service` in this module but they should be put in the module where they are used.



-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

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