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 2021/06/19 13:14:47 UTC

[GitHub] [dolphinscheduler] skymsg opened a new pull request #5665: [Improvement][Worker] Do not verify the status of yarn in ShellCommandExecutor 

skymsg opened a new pull request #5665:
URL: https://github.com/apache/dolphinscheduler/pull/5665


   … ShellCommandExecutor (#5564)
   
   <!--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
   Move the code that  verify the status of yarn in ShellCommandExecutor  to  AbstractYarnTask . 
   My shell command ouput log contains text that match the pattern APPLICATION_REGEX = "application_\\d+_\\d+"  but that task is not a yarn task. These yarn related code cause my shell command task failure.
   
   ## Brief change log
   
   <!--*(for example:)*
     - *Add maven-checkstyle-plugin to root pom.xml*
   -->
   ## Verify this pull request
   
   <!--*(Please pick either of the following options)*-->
   
   This pull request is code cleanup without any test coverage.
   


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

[GitHub] [dolphinscheduler] CalvinKirs commented on pull request #5665: [Improvement][Worker] Do not verify the status of yarn in ShellCommandExecutor

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


   deeply thx for your contribution, code style check fails.you can refer this doc(https://dolphinscheduler.apache.org/en-us/community/development/pull-request.html)


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

[GitHub] [dolphinscheduler] skymsg closed pull request #5665: [Improvement][Worker] Do not verify the status of yarn in ShellCommandExecutor

Posted by GitBox <gi...@apache.org>.
skymsg closed pull request #5665:
URL: https://github.com/apache/dolphinscheduler/pull/5665


   


-- 
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] CalvinKirs commented on pull request #5665: [Improvement][Worker] Do not verify the status of yarn in ShellCommandExecutor

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


   deeply thx for your contribution, code style check fails.you can refer this doc(https://dolphinscheduler.apache.org/en-us/community/development/pull-request.html)


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

[GitHub] [dolphinscheduler] ruanwenjun commented on a change in pull request #5665: [Improvement][Worker] Do not verify the status of yarn in ShellCommandExecutor

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



##########
File path: dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/AbstractCommandExecutor.java
##########
@@ -207,17 +203,10 @@ public CommandExecuteResult run(String execCommand) throws Exception {
 
         // if SHELL task exit
         if (status) {
-            // set appIds
-            List<String> appIds = getAppIds(taskExecutionContext.getLogPath());
-            result.setAppIds(String.join(Constants.COMMA, appIds));
 
             // SHELL task state
             result.setExitStatusCode(process.exitValue());
 
-            // if yarn task , yarn state is final state
-            if (process.exitValue() == 0) {
-                result.setExitStatusCode(isSuccessOfYarnState(appIds) ? EXIT_CODE_SUCCESS : EXIT_CODE_FAILURE);
-            }
         } else {
             logger.error("process has failure , exitStatusCode:{}, processExitValue:{}, ready to kill ...",
                  result.getExitStatusCode(), process.exitValue());

Review comment:
       We may also need to modify the kill method on line 214. In the current implementation, it will find `appliacationId` from log and kill the application on yarn, for normal tasks, we don't need to kill on yarn.




-- 
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 #5665: [Improvement][Worker] Do not verify the status of yarn in ShellCommandExecutor

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


   SonarCloud Quality Gate failed.
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C.png' alt='C' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=CODE_SMELL) [11 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0.png' alt='1.2%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=5665&metric=new_coverage&view=list) [1.2% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=5665&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=5665&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=5665&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] sonarcloud[bot] removed a comment on pull request #5665: [Improvement][Worker] Do not verify the status of yarn in ShellCommandExecutor

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #5665:
URL: https://github.com/apache/dolphinscheduler/pull/5665#issuecomment-868174454


   SonarCloud Quality Gate failed.
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C.png' alt='C' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=CODE_SMELL) [11 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0.png' alt='1.2%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=5665&metric=new_coverage&view=list) [1.2% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=5665&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=5665&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=5665&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] codecov-commenter edited a comment on pull request #5665: [Improvement][Worker] Do not verify the status of yarn in ShellCommandExecutor

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #5665:
URL: https://github.com/apache/dolphinscheduler/pull/5665#issuecomment-868172786


   # [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/5665?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 [#5665](https://codecov.io/gh/apache/dolphinscheduler/pull/5665?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3b4f06a) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/cf99df3de00ef63ee96b7ab00427c7385c42720a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cf99df3) will **decrease** coverage by `0.02%`.
   > The diff coverage is `7.04%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/dolphinscheduler/pull/5665/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/5665?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    #5665      +/-   ##
   ============================================
   - Coverage     45.34%   45.32%   -0.03%     
   + Complexity     3689     3682       -7     
   ============================================
     Files           607      607              
     Lines         24860    24850      -10     
     Branches       2825     2824       -1     
   ============================================
   - Hits          11274    11263      -11     
   - Misses        12504    12505       +1     
     Partials       1082     1082              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/5665?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...er/server/worker/task/AbstractCommandExecutor.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5665/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci90YXNrL0Fic3RyYWN0Q29tbWFuZEV4ZWN1dG9yLmphdmE=) | `22.01% <ø> (+2.01%)` | :arrow_up: |
   | [...duler/server/worker/task/CommandExecuteResult.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5665/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci90YXNrL0NvbW1hbmRFeGVjdXRlUmVzdWx0LmphdmE=) | `100.00% <ø> (ø)` | |
   | [...nscheduler/server/worker/task/datax/DataxTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5665/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci90YXNrL2RhdGF4L0RhdGF4VGFzay5qYXZh) | `55.31% <ø> (+0.23%)` | :arrow_up: |
   | [...cheduler/server/worker/task/python/PythonTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5665/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci90YXNrL3B5dGhvbi9QeXRob25UYXNrLmphdmE=) | `10.52% <ø> (+0.26%)` | :arrow_up: |
   | [...nscheduler/server/worker/task/shell/ShellTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5665/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci90YXNrL3NoZWxsL1NoZWxsVGFzay5qYXZh) | `6.89% <ø> (+0.11%)` | :arrow_up: |
   | [...scheduler/server/worker/task/AbstractYarnTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5665/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci90YXNrL0Fic3RyYWN0WWFyblRhc2suamF2YQ==) | `6.94% <7.04%> (-13.06%)` | :arrow_down: |
   | [...dolphinscheduler/remote/future/ResponseFuture.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5665/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.35% <0.00%> (-1.70%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/5665?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/5665?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 [cf99df3...3b4f06a](https://codecov.io/gh/apache/dolphinscheduler/pull/5665?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] CalvinKirs commented on pull request #5665: [Improvement][Worker] Do not verify the status of yarn in ShellCommandExecutor

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


   hi, how it is getting?
   


-- 
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 #5665: [Improvement][Worker] Do not verify the status of yarn in ShellCommandExecutor

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


   # [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/5665?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 [#5665](https://codecov.io/gh/apache/dolphinscheduler/pull/5665?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f39f6b4) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/a7682143ac70ab1aee6d1fd79c008ba9d48408dd?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a768214) will **decrease** coverage by `0.00%`.
   > The diff coverage is `5.71%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/dolphinscheduler/pull/5665/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/5665?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    #5665      +/-   ##
   ============================================
   - Coverage     45.31%   45.30%   -0.01%     
   + Complexity     3673     3669       -4     
   ============================================
     Files           607      607              
     Lines         24794    24784      -10     
     Branches       2803     2802       -1     
   ============================================
   - Hits          11235    11229       -6     
   + Misses        12489    12486       -3     
   + Partials       1070     1069       -1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/5665?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...er/server/worker/task/AbstractCommandExecutor.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5665/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci90YXNrL0Fic3RyYWN0Q29tbWFuZEV4ZWN1dG9yLmphdmE=) | `25.30% <ø> (+3.74%)` | :arrow_up: |
   | [...duler/server/worker/task/CommandExecuteResult.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5665/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci90YXNrL0NvbW1hbmRFeGVjdXRlUmVzdWx0LmphdmE=) | `100.00% <ø> (ø)` | |
   | [...nscheduler/server/worker/task/datax/DataxTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5665/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci90YXNrL2RhdGF4L0RhdGF4VGFzay5qYXZh) | `55.55% <ø> (+0.23%)` | :arrow_up: |
   | [...cheduler/server/worker/task/python/PythonTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5665/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci90YXNrL3B5dGhvbi9QeXRob25UYXNrLmphdmE=) | `10.81% <ø> (+0.28%)` | :arrow_up: |
   | [...nscheduler/server/worker/task/shell/ShellTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5665/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci90YXNrL3NoZWxsL1NoZWxsVGFzay5qYXZh) | `25.00% <ø> (+0.36%)` | :arrow_up: |
   | [...scheduler/server/worker/task/AbstractYarnTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5665/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci90YXNrL0Fic3RyYWN0WWFyblRhc2suamF2YQ==) | `6.94% <5.71%> (-13.06%)` | :arrow_down: |
   | [...eduler/server/worker/runner/TaskExecuteThread.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5665/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci9ydW5uZXIvVGFza0V4ZWN1dGVUaHJlYWQuamF2YQ==) | `59.50% <0.00%> (ø)` | |
   | [...olphinscheduler/plugin/alert/email/MailSender.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5665/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-ZG9scGhpbnNjaGVkdWxlci1hbGVydC1wbHVnaW4vZG9scGhpbnNjaGVkdWxlci1hbGVydC1lbWFpbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZG9scGhpbnNjaGVkdWxlci9wbHVnaW4vYWxlcnQvZW1haWwvTWFpbFNlbmRlci5qYXZh) | `74.30% <0.00%> (+1.38%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/5665?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/5665?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 [a768214...f39f6b4](https://codecov.io/gh/apache/dolphinscheduler/pull/5665?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.

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

[GitHub] [dolphinscheduler] skymsg commented on a change in pull request #5665: [Improvement][Worker] Do not verify the status of yarn in ShellCommandExecutor

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



##########
File path: dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/AbstractCommandExecutor.java
##########
@@ -207,17 +203,10 @@ public CommandExecuteResult run(String execCommand) throws Exception {
 
         // if SHELL task exit
         if (status) {
-            // set appIds
-            List<String> appIds = getAppIds(taskExecutionContext.getLogPath());
-            result.setAppIds(String.join(Constants.COMMA, appIds));
 
             // SHELL task state
             result.setExitStatusCode(process.exitValue());
 
-            // if yarn task , yarn state is final state
-            if (process.exitValue() == 0) {
-                result.setExitStatusCode(isSuccessOfYarnState(appIds) ? EXIT_CODE_SUCCESS : EXIT_CODE_FAILURE);
-            }
         } else {
             logger.error("process has failure , exitStatusCode:{}, processExitValue:{}, ready to kill ...",
                  result.getExitStatusCode(), process.exitValue());

Review comment:
       just remove the  killYarnJob call from kill method maybe  enough .
   ```java
     public static void kill(TaskExecutionContext taskExecutionContext) {
         try {
             int processId = taskExecutionContext.getProcessId();
             if (processId == 0) {
                 logger.error("process kill failed, process id :{}, task id:{}",
                         processId, taskExecutionContext.getTaskInstanceId());
                 return;
             }
   
             String pidsStr = getPidsStr(processId);
             if (StringUtils.isNotEmpty(pidsStr)) {
                 String cmd = String.format("kill -9 %s", pidsStr);
                 cmd = OSUtils.getSudoCmd(taskExecutionContext.getTenantCode(), cmd);
                 logger.info("process id:{}, cmd:{}", processId, cmd);
                 OSUtils.exeCmd(cmd);
             }
   
         } catch (Exception e) {
             logger.error("kill task failed", e);
         }
         // find log and kill yarn job
         killYarnJob(taskExecutionContext);
    }
   ```
   I found  the TaskExecuteThread would call the cancelApplication to kill the yarn job  when AbstractYarnTask handle  method throws Exception.




-- 
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 #5665: [Improvement][Worker] Do not verify the status of yarn in ShellCommandExecutor

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


   SonarCloud Quality Gate failed.
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C.png' alt='C' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=CODE_SMELL) [11 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5665&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0.png' alt='1.2%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=5665&metric=new_coverage&view=list) [1.2% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=5665&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=5665&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=5665&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.

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

[GitHub] [dolphinscheduler] skymsg commented on pull request #5665: [Improvement][Worker] Do not verify the status of yarn in ShellCommandExecutor

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


   Sorry , I have a busy schedule  recently.   I will try to fix these error  on the weekend.


-- 
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 change in pull request #5665: [Improvement][Worker] Do not verify the status of yarn in ShellCommandExecutor

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



##########
File path: dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/AbstractCommandExecutor.java
##########
@@ -207,17 +203,10 @@ public CommandExecuteResult run(String execCommand) throws Exception {
 
         // if SHELL task exit
         if (status) {
-            // set appIds
-            List<String> appIds = getAppIds(taskExecutionContext.getLogPath());
-            result.setAppIds(String.join(Constants.COMMA, appIds));
 
             // SHELL task state
             result.setExitStatusCode(process.exitValue());
 
-            // if yarn task , yarn state is final state
-            if (process.exitValue() == 0) {
-                result.setExitStatusCode(isSuccessOfYarnState(appIds) ? EXIT_CODE_SUCCESS : EXIT_CODE_FAILURE);
-            }
         } else {
             logger.error("process has failure , exitStatusCode:{}, processExitValue:{}, ready to kill ...",
                  result.getExitStatusCode(), process.exitValue());

Review comment:
       @skymsg In some case, the task may just exit with false status and won't throw an exception.




-- 
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] skymsg commented on a change in pull request #5665: [Improvement][Worker] Do not verify the status of yarn in ShellCommandExecutor

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



##########
File path: dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/AbstractCommandExecutor.java
##########
@@ -207,17 +203,10 @@ public CommandExecuteResult run(String execCommand) throws Exception {
 
         // if SHELL task exit
         if (status) {
-            // set appIds
-            List<String> appIds = getAppIds(taskExecutionContext.getLogPath());
-            result.setAppIds(String.join(Constants.COMMA, appIds));
 
             // SHELL task state
             result.setExitStatusCode(process.exitValue());
 
-            // if yarn task , yarn state is final state
-            if (process.exitValue() == 0) {
-                result.setExitStatusCode(isSuccessOfYarnState(appIds) ? EXIT_CODE_SUCCESS : EXIT_CODE_FAILURE);
-            }
         } else {
             logger.error("process has failure , exitStatusCode:{}, processExitValue:{}, ready to kill ...",
                  result.getExitStatusCode(), process.exitValue());

Review comment:
       the cancelApplication method  of  AbstractYarnTask  would call ProcessUtils.killYarnJob(taskExecutionContext)  for itself
   ```java
       public void cancelApplication(boolean status) throws Exception {
           cancel = true;
           // cancel process
           shellCommandExecutor.cancelApplication();
           TaskInstance taskInstance = processService.findTaskInstanceById(taskExecutionContext.getTaskInstanceId());
           if (status && taskInstance != null) {
               ProcessUtils.killYarnJob(taskExecutionContext);
           }
       }
   ```




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