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/02/26 04:40:25 UTC

[GitHub] [dolphinscheduler] Yao-MR opened a new pull request #8552: [Impreove][Core]remove no meaningful duplicated check code

Yao-MR opened a new pull request #8552:
URL: https://github.com/apache/dolphinscheduler/pull/8552


   ## Purpose of the pull request
   
   remove no meaningful duplicated check code
   
   ## Brief change log
   
   remove the duplicated check code
   
   ## Verify this pull request
   
   This pull request is code cleanup without any test coverage.
   
   This change added tests and can be verified as follows:
   
   
     - *remove the duplicated check 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: commits-unsubscribe@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] yimaixinchen commented on a change in pull request #8552: [Impreove][Core]remove no meaningful duplicated check code

Posted by GitBox <gi...@apache.org>.
yimaixinchen commented on a change in pull request #8552:
URL: https://github.com/apache/dolphinscheduler/pull/8552#discussion_r815426715



##########
File path: dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/MasterSchedulerService.java
##########
@@ -179,10 +179,6 @@ private void scheduleProcess() throws Exception {
     }
 
     private List<ProcessInstance> command2ProcessInstance(List<Command> commands) {
-        if (CollectionUtils.isEmpty(commands)) {
-            return null;
-        }
-
         List<ProcessInstance> processInstances = new ArrayList<>(commands.size());

Review comment:
       it returns NPE when commands is null




-- 
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] Yao-MR commented on a change in pull request #8552: [Impreove][Core]remove no meaningful duplicated check code

Posted by GitBox <gi...@apache.org>.
Yao-MR commented on a change in pull request #8552:
URL: https://github.com/apache/dolphinscheduler/pull/8552#discussion_r815536193



##########
File path: dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/MasterSchedulerService.java
##########
@@ -179,10 +179,6 @@ private void scheduleProcess() throws Exception {
     }
 
     private List<ProcessInstance> command2ProcessInstance(List<Command> commands) {
-        if (CollectionUtils.isEmpty(commands)) {
-            return null;
-        }
-
         List<ProcessInstance> processInstances = new ArrayList<>(commands.size());

Review comment:
       first we know the function is for transform the command to process instance,
   so we assue the command is not null, and also the we can know that the function who call the command2ProcessInstance have the predicate that the command is null or not null
   
   ![image](https://user-images.githubusercontent.com/8596901/155912368-d66b2b6e-375d-478d-b135-fe456f8e1e36.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: commits-unsubscribe@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] Yao-MR commented on a change in pull request #8552: [Impreove][Core]remove no meaningful duplicated check code

Posted by GitBox <gi...@apache.org>.
Yao-MR commented on a change in pull request #8552:
URL: https://github.com/apache/dolphinscheduler/pull/8552#discussion_r815536193



##########
File path: dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/MasterSchedulerService.java
##########
@@ -179,10 +179,6 @@ private void scheduleProcess() throws Exception {
     }
 
     private List<ProcessInstance> command2ProcessInstance(List<Command> commands) {
-        if (CollectionUtils.isEmpty(commands)) {
-            return null;
-        }
-
         List<ProcessInstance> processInstances = new ArrayList<>(commands.size());

Review comment:
       @yimaixinchen 
   
   first we know the function is for transform the command to process instance,
   so we assue the command is not null, and also the we can know that the function who call the command2ProcessInstance have the predicate that the command is null or not null
   
   ![image](https://user-images.githubusercontent.com/8596901/155912368-d66b2b6e-375d-478d-b135-fe456f8e1e36.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: 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 #8552: [Impreove][Core]remove no meaningful duplicated check code

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #8552:
URL: https://github.com/apache/dolphinscheduler/pull/8552#issuecomment-1051574322


   # [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/8552?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 [#8552](https://codecov.io/gh/apache/dolphinscheduler/pull/8552?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ad8ab20) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/ca6d148a1b48195a359a3ab29d449229a87dd05a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ca6d148) will **decrease** coverage by `0.16%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/dolphinscheduler/pull/8552/graphs/tree.svg?width=650&height=150&src=pr&token=bv9iXXRLi9&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/dolphinscheduler/pull/8552?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##                dev    #8552      +/-   ##
   ============================================
   - Coverage     45.41%   45.24%   -0.17%     
   - Complexity     4039     4040       +1     
   ============================================
     Files           688      690       +2     
     Lines         26773    26873     +100     
     Branches       2873     2884      +11     
   ============================================
   + Hits          12158    12160       +2     
   - Misses        13466    13564      +98     
     Partials       1149     1149              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/8552?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...r/plugin/registry/zookeeper/ZookeeperRegistry.java](https://codecov.io/gh/apache/dolphinscheduler/pull/8552/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-ZG9scGhpbnNjaGVkdWxlci1yZWdpc3RyeS9kb2xwaGluc2NoZWR1bGVyLXJlZ2lzdHJ5LXBsdWdpbnMvZG9scGhpbnNjaGVkdWxlci1yZWdpc3RyeS16b29rZWVwZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcGx1Z2luL3JlZ2lzdHJ5L3pvb2tlZXBlci9ab29rZWVwZXJSZWdpc3RyeS5qYXZh) | `48.21% <0.00%> (-7.15%)` | :arrow_down: |
   | [...cheduler/api/service/impl/ExecutorServiceImpl.java](https://codecov.io/gh/apache/dolphinscheduler/pull/8552/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvaW1wbC9FeGVjdXRvclNlcnZpY2VJbXBsLmphdmE=) | `41.96% <0.00%> (-5.14%)` | :arrow_down: |
   | [...inscheduler/api/controller/ExecutorController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/8552/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvRXhlY3V0b3JDb250cm9sbGVyLmphdmE=) | `40.00% <0.00%> (-1.94%)` | :arrow_down: |
   | [...dolphinscheduler/remote/future/ResponseFuture.java](https://codecov.io/gh/apache/dolphinscheduler/pull/8552/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-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL2Z1dHVyZS9SZXNwb25zZUZ1dHVyZS5qYXZh) | `81.96% <0.00%> (-1.64%)` | :arrow_down: |
   | [...lphinscheduler/service/process/ProcessService.java](https://codecov.io/gh/apache/dolphinscheduler/pull/8552/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3NlcnZpY2UvcHJvY2Vzcy9Qcm9jZXNzU2VydmljZS5qYXZh) | `31.88% <0.00%> (-0.08%)` | :arrow_down: |
   | [...a/org/apache/dolphinscheduler/dao/entity/User.java](https://codecov.io/gh/apache/dolphinscheduler/pull/8552/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-ZG9scGhpbnNjaGVkdWxlci1kYW8vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvZGFvL2VudGl0eS9Vc2VyLmphdmE=) | `74.13% <0.00%> (ø)` | |
   | [.../org/apache/dolphinscheduler/dao/entity/Queue.java](https://codecov.io/gh/apache/dolphinscheduler/pull/8552/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-ZG9scGhpbnNjaGVkdWxlci1kYW8vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvZGFvL2VudGl0eS9RdWV1ZS5qYXZh) | `51.61% <0.00%> (ø)` | |
   | [...org/apache/dolphinscheduler/dao/entity/Tenant.java](https://codecov.io/gh/apache/dolphinscheduler/pull/8552/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-ZG9scGhpbnNjaGVkdWxlci1kYW8vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvZGFvL2VudGl0eS9UZW5hbnQuamF2YQ==) | `72.72% <0.00%> (ø)` | |
   | [...rg/apache/dolphinscheduler/api/dto/gantt/Task.java](https://codecov.io/gh/apache/dolphinscheduler/pull/8552/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2R0by9nYW50dC9UYXNrLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...rg/apache/dolphinscheduler/dao/entity/Session.java](https://codecov.io/gh/apache/dolphinscheduler/pull/8552/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-ZG9scGhpbnNjaGVkdWxlci1kYW8vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvZGFvL2VudGl0eS9TZXNzaW9uLmphdmE=) | `41.93% <0.00%> (ø)` | |
   | ... and [37 more](https://codecov.io/gh/apache/dolphinscheduler/pull/8552/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) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/8552?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/8552?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [ca6d148...ad8ab20](https://codecov.io/gh/apache/dolphinscheduler/pull/8552?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] Yao-MR commented on pull request #8552: [Impreove][Core]remove no meaningful duplicated check code

Posted by GitBox <gi...@apache.org>.
Yao-MR commented on pull request #8552:
URL: https://github.com/apache/dolphinscheduler/pull/8552#issuecomment-1051570832


   just the dulicatede code
   
   <img width="647" alt="image" src="https://user-images.githubusercontent.com/8596901/155829260-948347bf-bfa7-4a9a-a3d5-686610aaf29e.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: commits-unsubscribe@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] caishunfeng merged pull request #8552: [Impreove][Master]remove no meaningful duplicated check code

Posted by GitBox <gi...@apache.org>.
caishunfeng merged pull request #8552:
URL: https://github.com/apache/dolphinscheduler/pull/8552


   


-- 
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] yimaixinchen commented on pull request #8552: [Impreove][Core]remove no meaningful duplicated check code

Posted by GitBox <gi...@apache.org>.
yimaixinchen commented on pull request #8552:
URL: https://github.com/apache/dolphinscheduler/pull/8552#issuecomment-1053526366


   Well I don't think it is duplicate.if it removes, the below codes that the commands.size return NPE .


-- 
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] Yao-MR commented on pull request #8552: [Impreove][Core]remove no meaningful duplicated check code

Posted by GitBox <gi...@apache.org>.
Yao-MR commented on pull request #8552:
URL: https://github.com/apache/dolphinscheduler/pull/8552#issuecomment-1057896600


   hi @caishunfeng  can you help check 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 #8552: [Impreove][Core]remove no meaningful duplicated check code

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #8552:
URL: https://github.com/apache/dolphinscheduler/pull/8552#issuecomment-1051578525


   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; ![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8552&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8552&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8552&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8552&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8552&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8552&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=8552&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=8552&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=8552&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8552&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8552&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=8552&resolved=false&types=CODE_SMELL)
   
   [![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=8552&metric=coverage&view=list) No Coverage information  
   [![No Duplication information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo-16px.png 'No Duplication information')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=8552&metric=duplicated_lines_density&view=list) No Duplication information
   
   


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