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/11/13 11:08:40 UTC
[GitHub] [dolphinscheduler] DarkAssassinator opened a new pull request, #12881: [Refactor] [Code smell] remove all spring bean params in WorkflowExecuteRunnable
DarkAssassinator opened a new pull request, #12881:
URL: https://github.com/apache/dolphinscheduler/pull/12881
<!--Thanks very much for contributing to Apache DolphinScheduler. Please review https://dolphinscheduler.apache.org/en-us/community/development/pull-request.html before opening a pull request.-->
## Purpose of the pull request
+ Code smell refactor : **Long Parameter List**.
## Brief change log
Long Parameter List in `WorkflowExecuteRunnable`, and the constructor of this class has more and more parameters, but all these params are Spring singleton bean, so we can remove these params.
## Verify this pull request
<!--*(Please pick either of the following options)*-->
This pull request is already covered by existing tests
--
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
[GitHub] [dolphinscheduler] ruanwenjun commented on a diff in pull request #12881: [Refactor] [Code smell] remove all spring bean params in WorkflowExecuteRunnable
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #12881:
URL: https://github.com/apache/dolphinscheduler/pull/12881#discussion_r1020895911
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/WorkflowExecuteRunnable.java:
##########
@@ -235,36 +236,19 @@ public class WorkflowExecuteRunnable implements Callable<WorkflowSubmitStatue> {
/**
* @param processInstance processInstance
- * @param processService processService
- * @param processInstanceDao processInstanceDao
- * @param nettyExecutorManager nettyExecutorManager
- * @param processAlertManager processAlertManager
- * @param masterConfig masterConfig
- * @param stateWheelExecuteThread stateWheelExecuteThread
*/
- public WorkflowExecuteRunnable(
- @NonNull ProcessInstance processInstance,
- @NonNull CommandService commandService,
- @NonNull ProcessService processService,
- @NonNull ProcessInstanceDao processInstanceDao,
- @NonNull NettyExecutorManager nettyExecutorManager,
- @NonNull ProcessAlertManager processAlertManager,
- @NonNull MasterConfig masterConfig,
- @NonNull StateWheelExecuteThread stateWheelExecuteThread,
- @NonNull CuringParamsService curingParamsService,
- @NonNull TaskInstanceDao taskInstanceDao,
- @NonNull TaskDefinitionLogDao taskDefinitionLogDao) {
- this.processService = processService;
- this.commandService = commandService;
- this.processInstanceDao = processInstanceDao;
+ public WorkflowExecuteRunnable(@NonNull ProcessInstance processInstance) {
+ this.processService = SpringApplicationContext.getBean(ProcessService.class);
Review Comment:
This is not a good idea to do this change, `WorkflowExecuteRunnable` is not a spring bean, it shouldn't rely spring.
--
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
[GitHub] [dolphinscheduler] ruanwenjun commented on pull request #12881: [Refactor] [Code smell] remove all spring bean params in WorkflowExecuteRunnable
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on PR #12881:
URL: https://github.com/apache/dolphinscheduler/pull/12881#issuecomment-1313527763
I will close this PR.
--
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
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #12881: [Refactor] [Code smell] remove all spring bean params in WorkflowExecuteRunnable
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12881:
URL: https://github.com/apache/dolphinscheduler/pull/12881#issuecomment-1312707886
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12881)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12881&metric=new_coverage&view=list) [85.7% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12881&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12881&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12881&metric=new_duplicated_lines_density&view=list)
--
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
[GitHub] [dolphinscheduler] ruanwenjun closed pull request #12881: [Refactor] [Code smell] remove all spring bean params in WorkflowExecuteRunnable
Posted by GitBox <gi...@apache.org>.
ruanwenjun closed pull request #12881: [Refactor] [Code smell] remove all spring bean params in WorkflowExecuteRunnable
URL: https://github.com/apache/dolphinscheduler/pull/12881
--
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
[GitHub] [dolphinscheduler] DarkAssassinator commented on pull request #12881: [Refactor] [Code smell] remove all spring bean params in WorkflowExecuteRunnable
Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on PR #12881:
URL: https://github.com/apache/dolphinscheduler/pull/12881#issuecomment-1313707251
> I will close this PR.
OK
--
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
[GitHub] [dolphinscheduler] codecov-commenter commented on pull request #12881: [Refactor] [Code smell] remove all spring bean params in WorkflowExecuteRunnable
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #12881:
URL: https://github.com/apache/dolphinscheduler/pull/12881#issuecomment-1312706679
# [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/12881?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#12881](https://codecov.io/gh/apache/dolphinscheduler/pull/12881?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (20b1688) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/feb077035e06b50c511b4baf9e659cbe83e86d56?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (feb0770) will **decrease** coverage by `0.01%`.
> The diff coverage is `83.33%`.
```diff
@@ Coverage Diff @@
## dev #12881 +/- ##
============================================
- Coverage 39.27% 39.26% -0.02%
+ Complexity 4232 4229 -3
============================================
Files 1051 1051
Lines 39849 39807 -42
Branches 4585 4557 -28
============================================
- Hits 15650 15629 -21
+ Misses 22427 22419 -8
+ Partials 1772 1759 -13
```
| [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/12881?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...server/master/runner/MasterSchedulerBootstrap.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12881/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9ydW5uZXIvTWFzdGVyU2NoZWR1bGVyQm9vdHN0cmFwLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [.../server/master/runner/WorkflowExecuteRunnable.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12881/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9ydW5uZXIvV29ya2Zsb3dFeGVjdXRlUnVubmFibGUuamF2YQ==) | `8.24% <90.90%> (+0.08%)` | :arrow_up: |
| [...erver/master/processor/queue/TaskEventService.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12881/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9wcm9jZXNzb3IvcXVldWUvVGFza0V2ZW50U2VydmljZS5qYXZh) | `69.64% <0.00%> (-10.72%)` | :arrow_down: |
| [...org/apache/dolphinscheduler/remote/utils/Host.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12881/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL3V0aWxzL0hvc3QuamF2YQ==) | `42.55% <0.00%> (-2.13%)` | :arrow_down: |
| [...hinscheduler/plugin/alert/script/ScriptSender.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12881/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hbGVydC9kb2xwaGluc2NoZWR1bGVyLWFsZXJ0LXBsdWdpbnMvZG9scGhpbnNjaGVkdWxlci1hbGVydC1zY3JpcHQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcGx1Z2luL2FsZXJ0L3NjcmlwdC9TY3JpcHRTZW5kZXIuamF2YQ==) | `70.58% <0.00%> (-1.64%)` | :arrow_down: |
| [...e/dolphinscheduler/remote/NettyRemotingClient.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12881/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL05ldHR5UmVtb3RpbmdDbGllbnQuamF2YQ==) | `51.38% <0.00%> (-1.39%)` | :arrow_down: |
| [...cheduler/plugin/alert/dingtalk/DingTalkSender.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12881/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hbGVydC9kb2xwaGluc2NoZWR1bGVyLWFsZXJ0LXBsdWdpbnMvZG9scGhpbnNjaGVkdWxlci1hbGVydC1kaW5ndGFsay9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZG9scGhpbnNjaGVkdWxlci9wbHVnaW4vYWxlcnQvZGluZ3RhbGsvRGluZ1RhbGtTZW5kZXIuamF2YQ==) | `34.13% <0.00%> (-0.78%)` | :arrow_down: |
| [...rver/master/runner/task/BlockingTaskProcessor.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12881/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9ydW5uZXIvdGFzay9CbG9ja2luZ1Rhc2tQcm9jZXNzb3IuamF2YQ==) | `75.86% <0.00%> (-0.55%)` | :arrow_down: |
| [...inscheduler/plugin/registry/etcd/EtcdRegistry.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12881/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1yZWdpc3RyeS9kb2xwaGluc2NoZWR1bGVyLXJlZ2lzdHJ5LXBsdWdpbnMvZG9scGhpbnNjaGVkdWxlci1yZWdpc3RyeS1ldGNkL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi9yZWdpc3RyeS9ldGNkL0V0Y2RSZWdpc3RyeS5qYXZh) | `50.69% <0.00%> (-0.35%)` | :arrow_down: |
| [.../dolphinscheduler/plugin/task/datax/DataxTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12881/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stZGF0YXgvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcGx1Z2luL3Rhc2svZGF0YXgvRGF0YXhUYXNrLmphdmE=) | `36.62% <0.00%> (-0.26%)` | :arrow_down: |
| ... and [19 more](https://codecov.io/gh/apache/dolphinscheduler/pull/12881/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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
[GitHub] [dolphinscheduler] ruanwenjun commented on a diff in pull request #12881: [Refactor] [Code smell] remove all spring bean params in WorkflowExecuteRunnable
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #12881:
URL: https://github.com/apache/dolphinscheduler/pull/12881#discussion_r1021007611
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/WorkflowExecuteRunnable.java:
##########
@@ -235,36 +236,19 @@ public class WorkflowExecuteRunnable implements Callable<WorkflowSubmitStatue> {
/**
* @param processInstance processInstance
- * @param processService processService
- * @param processInstanceDao processInstanceDao
- * @param nettyExecutorManager nettyExecutorManager
- * @param processAlertManager processAlertManager
- * @param masterConfig masterConfig
- * @param stateWheelExecuteThread stateWheelExecuteThread
*/
- public WorkflowExecuteRunnable(
- @NonNull ProcessInstance processInstance,
- @NonNull CommandService commandService,
- @NonNull ProcessService processService,
- @NonNull ProcessInstanceDao processInstanceDao,
- @NonNull NettyExecutorManager nettyExecutorManager,
- @NonNull ProcessAlertManager processAlertManager,
- @NonNull MasterConfig masterConfig,
- @NonNull StateWheelExecuteThread stateWheelExecuteThread,
- @NonNull CuringParamsService curingParamsService,
- @NonNull TaskInstanceDao taskInstanceDao,
- @NonNull TaskDefinitionLogDao taskDefinitionLogDao) {
- this.processService = processService;
- this.commandService = commandService;
- this.processInstanceDao = processInstanceDao;
+ public WorkflowExecuteRunnable(@NonNull ProcessInstance processInstance) {
+ this.processService = SpringApplicationContext.getBean(ProcessService.class);
Review Comment:
Refactor the contractor doesn't help to make `WorkflowExecuteRunnable ` clear.
--
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
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #12881: [Refactor] [Code smell] remove all spring bean params in WorkflowExecuteRunnable
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12881:
URL: https://github.com/apache/dolphinscheduler/pull/12881#issuecomment-1312708384
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12881)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12881&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12881&metric=new_coverage&view=list) [85.7% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12881&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12881&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12881&metric=new_duplicated_lines_density&view=list)
--
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
[GitHub] [dolphinscheduler] DarkAssassinator commented on a diff in pull request #12881: [Refactor] [Code smell] remove all spring bean params in WorkflowExecuteRunnable
Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on code in PR #12881:
URL: https://github.com/apache/dolphinscheduler/pull/12881#discussion_r1020899844
##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/WorkflowExecuteRunnable.java:
##########
@@ -235,36 +236,19 @@ public class WorkflowExecuteRunnable implements Callable<WorkflowSubmitStatue> {
/**
* @param processInstance processInstance
- * @param processService processService
- * @param processInstanceDao processInstanceDao
- * @param nettyExecutorManager nettyExecutorManager
- * @param processAlertManager processAlertManager
- * @param masterConfig masterConfig
- * @param stateWheelExecuteThread stateWheelExecuteThread
*/
- public WorkflowExecuteRunnable(
- @NonNull ProcessInstance processInstance,
- @NonNull CommandService commandService,
- @NonNull ProcessService processService,
- @NonNull ProcessInstanceDao processInstanceDao,
- @NonNull NettyExecutorManager nettyExecutorManager,
- @NonNull ProcessAlertManager processAlertManager,
- @NonNull MasterConfig masterConfig,
- @NonNull StateWheelExecuteThread stateWheelExecuteThread,
- @NonNull CuringParamsService curingParamsService,
- @NonNull TaskInstanceDao taskInstanceDao,
- @NonNull TaskDefinitionLogDao taskDefinitionLogDao) {
- this.processService = processService;
- this.commandService = commandService;
- this.processInstanceDao = processInstanceDao;
+ public WorkflowExecuteRunnable(@NonNull ProcessInstance processInstance) {
+ this.processService = SpringApplicationContext.getBean(ProcessService.class);
Review Comment:
> This is not a good idea to do this change, `WorkflowExecuteRunnable` is not a spring bean, it shouldn't rely spring.
Becuase current all `WorkflowExecuteRunnable` params are spring bean, so i think that we add `SpringApplicationContext` just change another way to get these params. If we should keep `WorkflowExecuteRunnable` clear, may we can migrate all DAO related method to other holder service, but this will lead to too scattered logic. Becuase more and more long params is a bad practies. WDYT.
--
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