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/05/12 07:46:31 UTC

[GitHub] [dolphinscheduler] zhuangchong opened a new pull request #5453: [Improvement-5452][Task] ds flink task support submit a PyFlink job via the CLI

zhuangchong opened a new pull request #5453:
URL: https://github.com/apache/dolphinscheduler/pull/5453


   ## Purpose of the pull request
   
   this close #5452 
   
   ds flink task support submit a PyFlink job via the CLI
   
   ```
   $ ./bin/flink run --python examples/python/table/batch/word_count.py
   ```
   


-- 
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] chengshiwen commented on a change in pull request #5453: [Improvement-5452][Task] ds flink task support submit a PyFlink job via the CLI

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



##########
File path: dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/utils/FlinkArgsUtils.java
##########
@@ -124,6 +124,10 @@ private FlinkArgsUtils() {
 
         ResourceInfo mainJar = param.getMainJar();
         if (mainJar != null) {
+            if (ProgramType.PYTHON == programType) {
+                // -py
+                args.add(Constants.FLINK_CLI_OPTION_PYTHON);
+            }

Review comment:
       These lines should not place here. It's better to modify them to:
   ```java
           if (programType != null) {
               if (programType == ProgramType.PYTHON) {
                   args.add(Constants.FLINK_PYTHON);        //-py
               } else if (StringUtils.isNotEmpty(mainClass)) {
                   args.add(Constants.FLINK_MAIN_CLASS);    //-c
                   args.add(param.getMainClass());          //main 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.

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

[GitHub] [dolphinscheduler] codecov-commenter commented on pull request #5453: [Improvement-5452][Task] ds flink task support submit a PyFlink job via the CLI

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


   # [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/5453?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 [#5453](https://codecov.io/gh/apache/dolphinscheduler/pull/5453?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (14ae297) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/d93a9dc2944856cd3d1c6cba669d30a7a44c10b4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d93a9dc) will **increase** coverage by `3.24%`.
   > The diff coverage is `44.80%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/dolphinscheduler/pull/5453/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/5453?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    #5453      +/-   ##
   ============================================
   + Coverage     45.21%   48.46%   +3.24%     
   - Complexity     3395     3855     +460     
   ============================================
     Files           538      598      +60     
     Lines         23053    24298    +1245     
     Branches       2687     2784      +97     
   ============================================
   + Hits          10423    11775    +1352     
   + Misses        11698    11446     -252     
   - Partials        932     1077     +145     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/5453?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...inscheduler/plugin/alert/email/EmailConstants.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5453/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-ZG9scGhpbnNjaGVkdWxlci1hbGVydC1wbHVnaW4vZG9scGhpbnNjaGVkdWxlci1hbGVydC1lbWFpbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZG9scGhpbnNjaGVkdWxlci9wbHVnaW4vYWxlcnQvZW1haWwvRW1haWxDb25zdGFudHMuamF2YQ==) | `87.50% <ø> (ø)` | `1.00 <0.00> (ø)` | |
   | [...eduler/plugin/alert/script/ScriptAlertChannel.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5453/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-ZG9scGhpbnNjaGVkdWxlci1hbGVydC1wbHVnaW4vZG9scGhpbnNjaGVkdWxlci1hbGVydC1zY3JpcHQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcGx1Z2luL2FsZXJ0L3NjcmlwdC9TY3JpcHRBbGVydENoYW5uZWwuamF2YQ==) | `16.66% <0.00%> (ø)` | `1.00 <0.00> (ø)` | |
   | [...org/apache/dolphinscheduler/alert/AlertServer.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5453/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-ZG9scGhpbnNjaGVkdWxlci1hbGVydC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZG9scGhpbnNjaGVkdWxlci9hbGVydC9BbGVydFNlcnZlci5qYXZh) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...che/dolphinscheduler/api/ApiApplicationServer.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5453/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL0FwaUFwcGxpY2F0aW9uU2VydmVyLmphdmE=) | `33.33% <0.00%> (ø)` | `1.00 <0.00> (ø)` | |
   | [...cheduler/api/controller/AccessTokenController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5453/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvQWNjZXNzVG9rZW5Db250cm9sbGVyLmphdmE=) | `6.66% <ø> (-1.67%)` | `1.00 <0.00> (-1.00)` | |
   | [...scheduler/api/controller/AlertGroupController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5453/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvQWxlcnRHcm91cENvbnRyb2xsZXIuamF2YQ==) | `8.00% <ø> (+2.73%)` | `2.00 <0.00> (ø)` | |
   | [.../api/controller/AlertPluginInstanceController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5453/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvQWxlcnRQbHVnaW5JbnN0YW5jZUNvbnRyb2xsZXIuamF2YQ==) | `7.69% <ø> (+1.97%)` | `2.00 <0.00> (ø)` | |
   | [...heduler/api/controller/DataAnalysisController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5453/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvRGF0YUFuYWx5c2lzQ29udHJvbGxlci5qYXZh) | `9.09% <ø> (ø)` | `1.00 <0.00> (-1.00)` | |
   | [...scheduler/api/controller/DataSourceController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5453/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvRGF0YVNvdXJjZUNvbnRyb2xsZXIuamF2YQ==) | `4.16% <0.00%> (-0.49%)` | `1.00 <0.00> (-1.00)` | |
   | [...inscheduler/api/controller/ExecutorController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5453/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=) | `8.33% <ø> (-2.78%)` | `1.00 <0.00> (-1.00)` | |
   | ... and [298 more](https://codecov.io/gh/apache/dolphinscheduler/pull/5453/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/5453?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/5453?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 [5556e47...14ae297](https://codecov.io/gh/apache/dolphinscheduler/pull/5453?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] chengshiwen commented on a change in pull request #5453: [Improvement-5452][Task] ds flink task support submit a PyFlink job via the CLI

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



##########
File path: dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/utils/FlinkArgsUtils.java
##########
@@ -124,6 +124,10 @@ private FlinkArgsUtils() {
 
         ResourceInfo mainJar = param.getMainJar();
         if (mainJar != null) {
+            if (ProgramType.PYTHON == programType) {
+                // -py
+                args.add(Constants.FLINK_CLI_OPTION_PYTHON);
+            }

Review comment:
       These lines should not place here. It's better to modify them to:
   ```java
           if (programType != null) {
               if (programType == ProgramType.PYTHON) {
                   args.add(Constants.FLINK_PYTHON);        // -py
               } else if (programType != ProgramType.PYTHON && StringUtils.isNotEmpty(mainClass)) {
                   args.add(Constants.FLINK_MAIN_CLASS);    //-c
                   args.add(param.getMainClass());          //main class
               }
           }
   ```

##########
File path: dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/utils/FlinkArgsUtils.java
##########
@@ -124,6 +124,10 @@ private FlinkArgsUtils() {
 
         ResourceInfo mainJar = param.getMainJar();
         if (mainJar != null) {
+            if (ProgramType.PYTHON == programType) {
+                // -py
+                args.add(Constants.FLINK_CLI_OPTION_PYTHON);
+            }

Review comment:
       These lines should not place here. It's better to modify them to:
   ```java
           if (programType != null) {
               if (programType == ProgramType.PYTHON) {
                   args.add(Constants.FLINK_PYTHON);        //-py
               } else if (programType != ProgramType.PYTHON && StringUtils.isNotEmpty(mainClass)) {
                   args.add(Constants.FLINK_MAIN_CLASS);    //-c
                   args.add(param.getMainClass());          //main 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.

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

[GitHub] [dolphinscheduler] chengshiwen commented on a change in pull request #5453: [Improvement-5452][Task] ds flink task support submit a PyFlink job via the CLI

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



##########
File path: dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/utils/FlinkArgsUtils.java
##########
@@ -124,6 +124,10 @@ private FlinkArgsUtils() {
 
         ResourceInfo mainJar = param.getMainJar();
         if (mainJar != null) {
+            if (ProgramType.PYTHON == programType) {
+                // -py
+                args.add(Constants.FLINK_CLI_OPTION_PYTHON);
+            }

Review comment:
       These lines should not place here. It's better to modify:
   ```java
           if (programType != null) {
               if (programType == ProgramType.PYTHON) {
                   args.add(Constants.FLINK_PYTHON);        // -py
               } else if (programType != ProgramType.PYTHON && StringUtils.isNotEmpty(mainClass)) {
                   args.add(Constants.FLINK_MAIN_CLASS);    //-c
                   args.add(param.getMainClass());          //main class
               }
           }
   ```

##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/Constants.java
##########
@@ -870,6 +870,7 @@ private Constants() {
     public static final String FLINK_MAIN_CLASS = "-c";
     public static final String FLINK_PARALLELISM = "-p";
     public static final String FLINK_SHUTDOWN_ON_ATTACHED_EXIT = "-sae";
+    public static final String FLINK_CLI_OPTION_PYTHON = "-py";

Review comment:
       I think maybe `FLINK_PYTHON` is better




-- 
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] zhuangchong commented on pull request #5453: [Improvement-5452][Task] ds flink task support submit a PyFlink job via the CLI

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


   > @zhuangchong Hi, how is it getting?
   
   done.


-- 
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] sonarcloud[bot] commented on pull request #5453: [Improvement-5452][Task] ds flink task support submit a PyFlink job via the CLI

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


   Kudos, SonarCloud Quality Gate passed!
   
   [<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=5453&resolved=false&types=BUG) [<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=5453&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5453&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=5453&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=5453&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5453&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=5453&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=5453&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=5453&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=5453&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=5453&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5453&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='72.7%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=5453&metric=new_coverage&view=list) [72.7% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=5453&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=5453&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=5453&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] sonarcloud[bot] commented on pull request #5453: [Improvement-5452][Task] ds flink task support submit a PyFlink job via the CLI

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


   Kudos, SonarCloud Quality Gate passed!
   
   [<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=5453&resolved=false&types=BUG) [<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=5453&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5453&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=5453&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=5453&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5453&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=5453&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=5453&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=5453&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=5453&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=5453&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=5453&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='50.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=5453&metric=new_coverage&view=list) [50.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=5453&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=5453&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=5453&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] chengshiwen commented on a change in pull request #5453: [Improvement-5452][Task] ds flink task support submit a PyFlink job via the CLI

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



##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/Constants.java
##########
@@ -870,6 +870,7 @@ private Constants() {
     public static final String FLINK_MAIN_CLASS = "-c";
     public static final String FLINK_PARALLELISM = "-p";
     public static final String FLINK_SHUTDOWN_ON_ATTACHED_EXIT = "-sae";
+    public static final String FLINK_CLI_OPTION_PYTHON = "-py";

Review comment:
       I think maybe `FLINK_PYTHON` is shorter and better




-- 
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] codecov-commenter edited a comment on pull request #5453: [Improvement-5452][Task] ds flink task support submit a PyFlink job via the CLI

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


   # [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/5453?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 [#5453](https://codecov.io/gh/apache/dolphinscheduler/pull/5453?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (80f62f0) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/9d70c7e534aa10e729d564f14a1126f623cd1035?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9d70c7e) will **decrease** coverage by `0.01%`.
   > The diff coverage is `40.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/dolphinscheduler/pull/5453/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/5453?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    #5453      +/-   ##
   ============================================
   - Coverage     45.33%   45.31%   -0.02%     
   + Complexity     3674     3670       -4     
   ============================================
     Files           607      607              
     Lines         24780    24780              
     Branches       2801     2801              
   ============================================
   - Hits          11234    11230       -4     
     Misses        12479    12479              
   - Partials       1067     1071       +4     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/5453?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [.../org/apache/dolphinscheduler/common/Constants.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5453/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-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL0NvbnN0YW50cy5qYXZh) | `80.00% <ø> (ø)` | |
   | [.../dolphinscheduler/server/utils/FlinkArgsUtils.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5453/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3V0aWxzL0ZsaW5rQXJnc1V0aWxzLmphdmE=) | `83.05% <40.00%> (-2.92%)` | :arrow_down: |
   | [...er/master/dispatch/host/assign/RandomSelector.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5453/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9kaXNwYXRjaC9ob3N0L2Fzc2lnbi9SYW5kb21TZWxlY3Rvci5qYXZh) | `77.77% <0.00%> (-5.56%)` | :arrow_down: |
   | [...dolphinscheduler/remote/future/ResponseFuture.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5453/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: |
   | [...er/server/worker/task/AbstractCommandExecutor.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5453/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=) | `21.55% <0.00%> (-0.92%)` | :arrow_down: |
   | [...phinscheduler/service/registry/RegistryCenter.java](https://codecov.io/gh/apache/dolphinscheduler/pull/5453/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3NlcnZpY2UvcmVnaXN0cnkvUmVnaXN0cnlDZW50ZXIuamF2YQ==) | `26.22% <0.00%> (+0.83%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/5453?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/5453?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 [9d70c7e...80f62f0](https://codecov.io/gh/apache/dolphinscheduler/pull/5453?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] Dawn-K commented on a change in pull request #5453: [Improvement-5452][Task] ds flink task support submit a PyFlink job via the CLI

Posted by GitBox <gi...@apache.org>.
Dawn-K commented on a change in pull request #5453:
URL: https://github.com/apache/dolphinscheduler/pull/5453#discussion_r646371013



##########
File path: dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/utils/FlinkArgsUtils.java
##########
@@ -124,6 +124,10 @@ private FlinkArgsUtils() {
 
         ResourceInfo mainJar = param.getMainJar();
         if (mainJar != null) {
+            if (ProgramType.PYTHON == programType) {
+                // -py
+                args.add(Constants.FLINK_CLI_OPTION_PYTHON);
+            }

Review comment:
       > These lines should not place here. It's better to modify them to:
   > 
   > ```java
   >         if (programType != null) {
   >             if (programType == ProgramType.PYTHON) {
   >                 args.add(Constants.FLINK_PYTHON);        //-py
   >             } else if (programType != ProgramType.PYTHON && StringUtils.isNotEmpty(mainClass)) {
   >                 args.add(Constants.FLINK_MAIN_CLASS);    //-c
   >                 args.add(param.getMainClass());          //main class
   >             }
   >         }
   > ```
   
   Hello, I think the `programType != ProgramType.PYTHON` in the else-if brackets is not necessary. Because once this sentence is judged, this item must be true.




-- 
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 #5453: [Improvement-5452][Task] ds flink task support submit a PyFlink job via the CLI

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


   @zhuangchong Hi, how is it 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.

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