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/10/16 06:47:42 UTC

[GitHub] [dolphinscheduler] DarkAssassinator opened a new pull request, #12388: [Fix-12368] [Task Plugin] Fix DataX plugin NPE issue.

DarkAssassinator opened a new pull request, #12388:
URL: https://github.com/apache/dolphinscheduler/pull/12388

   <!--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
   
   + This PR will close #12368 
   
   ## Brief change log
   
   + Fix NPE 
   + Refactor some cases
   + Add DataxTask UT
   
   ## Verify this pull request
   + This change added tests
   + UT do not cover all provate methods.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] DarkAssassinator commented on a diff in pull request #12388: [Fix-12368] [Task Plugin] Fix DataX plugin NPE issue.

Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on code in PR #12388:
URL: https://github.com/apache/dolphinscheduler/pull/12388#discussion_r996437584


##########
dolphinscheduler-task-plugin/dolphinscheduler-task-datax/src/main/java/org/apache/dolphinscheduler/plugin/task/datax/DataxTask.java:
##########
@@ -393,18 +392,17 @@ private String buildShellCommandFile(String jobConfigFilePath, Map<String, Prope
         }
 
         // datax python command
-        StringBuilder sbr = new StringBuilder();

Review Comment:
   @EricGao888 @fuchanghai Sure, because JVM internally compiler will geneate axactly the same bytecode and `String +` and `StringBuilder.append` will have the same performance and the same memory usage, but String will be easier to read. We can get a readability benefit without any performace compromises. The replacement does not result in significant performance drawback on modern JVMs.
   Please check the following screenshot, the modern JVM will pptimize `String +` to `StringBuilder.append`, so they are same.
   ![image](https://user-images.githubusercontent.com/20518339/196035542-fabdf90b-2b3c-4efe-882d-d05c57a76c8e.png)
   If just compare `String` with `StringBuilder`, on most cases, using String is more practical and logical. It is not always better to concatenate string with plus sign (+). For example, StringBuilder's append method is more logical if you change your string in a loop because of its mutability. 
   For more details, please check [JLS: 15.18.1. String Concatenation Operator +](https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.18.1)
   And current IDEA is so smart, it will suggests you to change it for better code readability. It is equivalent to the optimization of the compiler, so IDEA detects that the two are equivalent. In order to make the code more concise, it will give some tips. Please check [IDEA:Convert StringBuilder to String concatenation suggestion](https://intellij-support.jetbrains.com/hc/en-us/community/posts/115000093250-Convert-StringBuilder-to-String-concatenation-suggestion)
   
   
   



-- 
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] EricGao888 commented on a diff in pull request #12388: [Fix-12368] [Task Plugin] Fix DataX plugin NPE issue.

Posted by GitBox <gi...@apache.org>.
EricGao888 commented on code in PR #12388:
URL: https://github.com/apache/dolphinscheduler/pull/12388#discussion_r996428558


##########
dolphinscheduler-task-plugin/dolphinscheduler-task-datax/src/main/java/org/apache/dolphinscheduler/plugin/task/datax/DataxTask.java:
##########
@@ -393,18 +392,17 @@ private String buildShellCommandFile(String jobConfigFilePath, Map<String, Prope
         }
 
         // datax python command
-        StringBuilder sbr = new StringBuilder();

Review Comment:
   May I ask why you change to use `String` here?



-- 
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] fuchanghai commented on a diff in pull request #12388: [Fix-12368] [Task Plugin] Fix DataX plugin NPE issue.

Posted by GitBox <gi...@apache.org>.
fuchanghai commented on code in PR #12388:
URL: https://github.com/apache/dolphinscheduler/pull/12388#discussion_r996418817


##########
dolphinscheduler-task-plugin/dolphinscheduler-task-datax/src/main/java/org/apache/dolphinscheduler/plugin/task/datax/DataxTask.java:
##########
@@ -393,18 +392,17 @@ private String buildShellCommandFile(String jobConfigFilePath, Map<String, Prope
         }
 
         // datax python command
-        StringBuilder sbr = new StringBuilder();
-        sbr.append(getPythonCommand());
-        sbr.append(" ");
-        sbr.append(DATAX_PATH);
-        sbr.append(" ");
-        sbr.append(loadJvmEnv(dataXParameters));
-        sbr.append(addCustomParameters(paramsMap));
-        sbr.append(" ");
-        sbr.append(jobConfigFilePath);
+        String sbr = getPythonCommand() +

Review Comment:
    if `sbr` use type of String, will it cause the JVM to generate multiple objects?



-- 
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 #12388: [Fix-12368] [Task Plugin] Fix DataX plugin NPE issue.

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

   # [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/12388?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 [#12388](https://codecov.io/gh/apache/dolphinscheduler/pull/12388?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a46c54a) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/1e792186d0b1e660dc2299b46827a46c59d393f6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1e79218) will **increase** coverage by `0.29%`.
   > The diff coverage is `63.63%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##                dev   #12388      +/-   ##
   ============================================
   + Coverage     38.93%   39.22%   +0.29%     
   - Complexity     4175     4212      +37     
   ============================================
     Files          1040     1040              
     Lines         38802    38799       -3     
     Branches       4462     4462              
   ============================================
   + Hits          15107    15219     +112     
   + Misses        21927    21804     -123     
   - Partials       1768     1776       +8     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/12388?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [.../dolphinscheduler/plugin/task/datax/DataxTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12388/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stZGF0YXgvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcGx1Z2luL3Rhc2svZGF0YXgvRGF0YXhUYXNrLmphdmE=) | `36.77% <63.63%> (+36.77%)` | :arrow_up: |
   | [...r/plugin/task/sqoop/parameter/SqoopParameters.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12388/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-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stc3Fvb3Avc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcGx1Z2luL3Rhc2svc3Fvb3AvcGFyYW1ldGVyL1Nxb29wUGFyYW1ldGVycy5qYXZh) | `55.12% <0.00%> (-1.29%)` | :arrow_down: |
   | [...e/dolphinscheduler/remote/NettyRemotingClient.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12388/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL05ldHR5UmVtb3RpbmdDbGllbnQuamF2YQ==) | `52.14% <0.00%> (-0.72%)` | :arrow_down: |
   | [...r/plugin/task/datax/DataxTaskExecutionContext.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12388/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-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stZGF0YXgvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcGx1Z2luL3Rhc2svZGF0YXgvRGF0YXhUYXNrRXhlY3V0aW9uQ29udGV4dC5qYXZh) | `5.00% <0.00%> (+5.00%)` | :arrow_up: |
   | [...inscheduler/plugin/task/datax/DataxParameters.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12388/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-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stZGF0YXgvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcGx1Z2luL3Rhc2svZGF0YXgvRGF0YXhQYXJhbWV0ZXJzLmphdmE=) | `68.91% <0.00%> (+32.43%)` | :arrow_up: |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #12388: [Fix-12368] [Task Plugin] Fix DataX plugin NPE issue.

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

   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')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12388)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12388&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=12388&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12388&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=12388&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=12388&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12388&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=12388&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=12388&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12388&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=12388&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=12388&resolved=false&types=CODE_SMELL) [16 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12388&resolved=false&types=CODE_SMELL)
   
   [![72.2%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '72.2%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12388&metric=new_coverage&view=list) [72.2% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12388&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12388&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12388&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] EricGao888 commented on a diff in pull request #12388: [Fix-12368] [Task Plugin] Fix DataX plugin NPE issue.

Posted by GitBox <gi...@apache.org>.
EricGao888 commented on code in PR #12388:
URL: https://github.com/apache/dolphinscheduler/pull/12388#discussion_r996443506


##########
dolphinscheduler-task-plugin/dolphinscheduler-task-datax/src/main/java/org/apache/dolphinscheduler/plugin/task/datax/DataxTask.java:
##########
@@ -393,18 +392,17 @@ private String buildShellCommandFile(String jobConfigFilePath, Map<String, Prope
         }
 
         // datax python command
-        StringBuilder sbr = new StringBuilder();

Review Comment:
   Make sense to me. Nice work and thx for the explanation.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] DarkAssassinator commented on a diff in pull request #12388: [Fix-12368] [Task Plugin] Fix DataX plugin NPE issue.

Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on code in PR #12388:
URL: https://github.com/apache/dolphinscheduler/pull/12388#discussion_r996437584


##########
dolphinscheduler-task-plugin/dolphinscheduler-task-datax/src/main/java/org/apache/dolphinscheduler/plugin/task/datax/DataxTask.java:
##########
@@ -393,18 +392,17 @@ private String buildShellCommandFile(String jobConfigFilePath, Map<String, Prope
         }
 
         // datax python command
-        StringBuilder sbr = new StringBuilder();

Review Comment:
   @EricGao888 @fuchanghai Sure, because JVM internally compiler will geneate axactly the same bytecode and `String +` and `StringBuilder.append` will have the same performance and the same memory usage, but String will be easier to read. We can get a readability benefit without any performace compromises. The replacement does not result in significant performance drawback on modern JVMs.
   Please check the following screenshot, the modern JVM will pptimize `String +` to `StringBuilder.append`, so they are same.
   ![image](https://user-images.githubusercontent.com/20518339/196035542-fabdf90b-2b3c-4efe-882d-d05c57a76c8e.png)
   If just compare `String` with `StringBuilder`, on most cases, using String is more practical and logical. It is not always better to concatenate string with plus sign (+). For example, StringBuilder's append method is more logical if you change your string in a loop because of its mutability. 
   For more details, please check [JLS: 15.18.1. String Concatenation Operator +](https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.18.1)
   And current IDEA is so smart, it will suggests you to change it for better code readability. It is equivalent to the optimization of the compiler, so IDEA detects that the two are equivalent. In order to make the code more concise, it will give some tips. thx
   
   
   



-- 
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 #12388: [Fix-12368] [Task Plugin] Fix DataX plugin NPE issue.

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

   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')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12388)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12388&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=12388&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12388&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=12388&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=12388&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12388&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=12388&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=12388&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12388&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=12388&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=12388&resolved=false&types=CODE_SMELL) [16 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12388&resolved=false&types=CODE_SMELL)
   
   [![72.2%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '72.2%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12388&metric=new_coverage&view=list) [72.2% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12388&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12388&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12388&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] fuchanghai commented on a diff in pull request #12388: [Fix-12368] [Task Plugin] Fix DataX plugin NPE issue.

Posted by GitBox <gi...@apache.org>.
fuchanghai commented on code in PR #12388:
URL: https://github.com/apache/dolphinscheduler/pull/12388#discussion_r996418618


##########
dolphinscheduler-task-plugin/dolphinscheduler-task-datax/src/main/java/org/apache/dolphinscheduler/plugin/task/datax/DataxTask.java:
##########
@@ -393,18 +392,17 @@ private String buildShellCommandFile(String jobConfigFilePath, Map<String, Prope
         }
 
         // datax python command
-        StringBuilder sbr = new StringBuilder();
-        sbr.append(getPythonCommand());
-        sbr.append(" ");
-        sbr.append(DATAX_PATH);
-        sbr.append(" ");
-        sbr.append(loadJvmEnv(dataXParameters));
-        sbr.append(addCustomParameters(paramsMap));
-        sbr.append(" ");
-        sbr.append(jobConfigFilePath);
+        String sbr = getPythonCommand() +
+                " " +

Review Comment:
   if ```sbr``` use type of String, will it cause the JVM to generate multiple objects?



-- 
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] zhuangchong merged pull request #12388: [Fix-12368] [Task Plugin] Fix DataX plugin NPE issue.

Posted by GitBox <gi...@apache.org>.
zhuangchong merged PR #12388:
URL: https://github.com/apache/dolphinscheduler/pull/12388


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