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 2020/09/15 04:42:05 UTC

[GitHub] [incubator-dolphinscheduler] zhuangchong opened a new pull request #3746: [fix-3745][server] server get tasktype NPE exception

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


   ## What is the purpose of the pull request
   #3745 
   ```
   switch (EnumUtils.getEnum(TaskType.class,taskExecutionContext.getTaskType()))
   ```
   When the task type does not exist in the enumeration class TaskType, EnumUtils.getEnum() returns null, and switch will report a null pointer exception
   
   ## Brief change log
   
   1.TaskParametersUtils  switch tasktype param is not null
   1.TaskManager switch tasktype  param is not null
   
   ## Verify this pull request
   
   This pull request is already covered by existing tests, such as *(please describe tests)*.
   
   TaskParametersUtilsTest
   TaskManagerTest
   


----------------------------------------------------------------
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] [incubator-dolphinscheduler] zhuangchong commented on a change in pull request #3746: [fix-3745][server] server get tasktype NPE exception

Posted by GitBox <gi...@apache.org>.
zhuangchong commented on a change in pull request #3746:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3746#discussion_r489302105



##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/TaskParametersUtils.java
##########
@@ -55,41 +55,42 @@ private TaskParametersUtils() {
      * @return task parameters
      */
     public static AbstractParameters getParameters(String taskType, String parameter) {
-        try {
-            switch (EnumUtils.getEnum(TaskType.class, taskType)) {
-                case SUB_PROCESS:
-                    return JSONUtils.parseObject(parameter, SubProcessParameters.class);
-                case SHELL:
-                case WATERDROP:
-                    return JSONUtils.parseObject(parameter, ShellParameters.class);
-                case PROCEDURE:
-                    return JSONUtils.parseObject(parameter, ProcedureParameters.class);
-                case SQL:
-                    return JSONUtils.parseObject(parameter, SqlParameters.class);
-                case MR:
-                    return JSONUtils.parseObject(parameter, MapreduceParameters.class);
-                case SPARK:
-                    return JSONUtils.parseObject(parameter, SparkParameters.class);
-                case PYTHON:
-                    return JSONUtils.parseObject(parameter, PythonParameters.class);
-                case DEPENDENT:
-                    return JSONUtils.parseObject(parameter, DependentParameters.class);
-                case FLINK:
-                    return JSONUtils.parseObject(parameter, FlinkParameters.class);
-                case HTTP:
-                    return JSONUtils.parseObject(parameter, HttpParameters.class);
-                case DATAX:
-                    return JSONUtils.parseObject(parameter, DataxParameters.class);
-                case CONDITIONS:
-                    return JSONUtils.parseObject(parameter, ConditionsParameters.class);
-                case SQOOP:
-                    return JSONUtils.parseObject(parameter, SqoopParameters.class);
-                default:
-                    return null;
-            }
-        } catch (Exception e) {
-            logger.error(e.getMessage(), e);
+        TaskType anEnum = EnumUtils.getEnum(TaskType.class, taskType);
+        if (anEnum == null) {
+            logger.error("not support task type: {}", taskType);
+            return null;

Review comment:
       This method is called by multiple methods in the project, which has a big impact. It still returns null according to the original logic of the method. This time, we mainly deal with the switch null pointer problem.
   
   ---
   这个方法被项目里面多个方法调用，影响大，还是按方法原逻辑返回null,本次主要处理switch 空指针问题。




----------------------------------------------------------------
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] [incubator-dolphinscheduler] yangyichao-mango commented on a change in pull request #3746: [fix-3745][server] server get tasktype NPE exception

Posted by GitBox <gi...@apache.org>.
yangyichao-mango commented on a change in pull request #3746:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3746#discussion_r489160227



##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/TaskParametersUtils.java
##########
@@ -55,41 +55,42 @@ private TaskParametersUtils() {
      * @return task parameters
      */
     public static AbstractParameters getParameters(String taskType, String parameter) {
-        try {
-            switch (EnumUtils.getEnum(TaskType.class, taskType)) {
-                case SUB_PROCESS:
-                    return JSONUtils.parseObject(parameter, SubProcessParameters.class);
-                case SHELL:
-                case WATERDROP:
-                    return JSONUtils.parseObject(parameter, ShellParameters.class);
-                case PROCEDURE:
-                    return JSONUtils.parseObject(parameter, ProcedureParameters.class);
-                case SQL:
-                    return JSONUtils.parseObject(parameter, SqlParameters.class);
-                case MR:
-                    return JSONUtils.parseObject(parameter, MapreduceParameters.class);
-                case SPARK:
-                    return JSONUtils.parseObject(parameter, SparkParameters.class);
-                case PYTHON:
-                    return JSONUtils.parseObject(parameter, PythonParameters.class);
-                case DEPENDENT:
-                    return JSONUtils.parseObject(parameter, DependentParameters.class);
-                case FLINK:
-                    return JSONUtils.parseObject(parameter, FlinkParameters.class);
-                case HTTP:
-                    return JSONUtils.parseObject(parameter, HttpParameters.class);
-                case DATAX:
-                    return JSONUtils.parseObject(parameter, DataxParameters.class);
-                case CONDITIONS:
-                    return JSONUtils.parseObject(parameter, ConditionsParameters.class);
-                case SQOOP:
-                    return JSONUtils.parseObject(parameter, SqoopParameters.class);
-                default:
-                    return null;
-            }
-        } catch (Exception e) {
-            logger.error(e.getMessage(), e);
+        TaskType anEnum = EnumUtils.getEnum(TaskType.class, taskType);
+        if (anEnum == null) {
+            logger.error("not support task type: {}", taskType);
+            return null;

Review comment:
       > Why not throw InvalidArgumentException?
   
   +1， fail fast will be better.

##########
File path: dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/TaskManager.java
##########
@@ -69,8 +74,9 @@ public static AbstractTask newTask(TaskExecutionContext taskExecutionContext, Lo
             case SQOOP:
                 return new SqoopTask(taskExecutionContext, logger);
             default:
-                logger.error("unsupport task type: {}", taskExecutionContext.getTaskType());
-                throw new IllegalArgumentException("not support task type");
+                String msg = String.format("not support task type: %s", taskExecutionContext.getTaskType());

Review comment:
       it will be better to use log pattern，not string format.




----------------------------------------------------------------
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] [incubator-dolphinscheduler] Technoboy- merged pull request #3746: [fix-3745][server] server get tasktype NPE exception

Posted by GitBox <gi...@apache.org>.
Technoboy- merged pull request #3746:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3746


   


----------------------------------------------------------------
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] [incubator-dolphinscheduler] sonarcloud[bot] commented on pull request #3746: [fix-3745][server] server get tasktype NPE exception

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


   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=3746&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=3746&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3746&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=3746&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=3746&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3746&resolved=false&types=VULNERABILITY) (and [<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/issues?id=apache-dolphinscheduler&pullRequest=3746&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/proje
 ct/issues?id=apache-dolphinscheduler&pullRequest=3746&resolved=false&types=SECURITY_HOTSPOT) to review)  
   [<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=3746&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=3746&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3746&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='76.7%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3746&metric=new_coverage&view=list) [76.7% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3746&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=3746&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3746&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] [incubator-dolphinscheduler] sonarcloud[bot] removed a comment on pull request #3746: [fix-3745][server] server get tasktype NPE exception

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


   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=3746&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=3746&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3746&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=3746&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=3746&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3746&resolved=false&types=VULNERABILITY) (and [<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/issues?id=apache-dolphinscheduler&pullRequest=3746&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/proje
 ct/issues?id=apache-dolphinscheduler&pullRequest=3746&resolved=false&types=SECURITY_HOTSPOT) to review)  
   [<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=3746&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=3746&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3746&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='76.7%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3746&metric=new_coverage&view=list) [76.7% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3746&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=3746&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3746&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] [incubator-dolphinscheduler] sonarcloud[bot] removed a comment on pull request #3746: [fix-3745][server] server get tasktype NPE exception

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


   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=3746&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=3746&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3746&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=3746&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=3746&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3746&resolved=false&types=VULNERABILITY) (and [<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/issues?id=apache-dolphinscheduler&pullRequest=3746&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/proje
 ct/issues?id=apache-dolphinscheduler&pullRequest=3746&resolved=false&types=SECURITY_HOTSPOT) to review)  
   [<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=3746&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=3746&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3746&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='76.7%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3746&metric=new_coverage&view=list) [76.7% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3746&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=3746&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3746&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] [incubator-dolphinscheduler] Technoboy- commented on a change in pull request #3746: [fix-3745][server] server get tasktype NPE exception

Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #3746:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3746#discussion_r489160211



##########
File path: dolphinscheduler-server/src/test/java/org/apache/dolphinscheduler/server/worker/task/TaskManagerTest.java
##########
@@ -95,9 +97,19 @@ public void testNewTask() {
         Assert.assertNotNull(TaskManager.newTask(taskExecutionContext,taskLogger));
         taskExecutionContext.setTaskType("SQOOP");
         Assert.assertNotNull(TaskManager.newTask(taskExecutionContext,taskLogger));
-        //taskExecutionContext.setTaskType(null);
-        //Assert.assertNull(TaskManager.newTask(taskExecutionContext,taskLogger));
-        //taskExecutionContext.setTaskType("XXX");
-        //Assert.assertNotNull(TaskManager.newTask(taskExecutionContext,taskLogger));
+        try {
+            taskExecutionContext.setTaskType(null);
+            TaskManager.newTask(taskExecutionContext,taskLogger);
+        } catch (Exception e) {
+            logger.error(e.getMessage());
+        }
+
+        try {
+            taskExecutionContext.setTaskType("XXX");

Review comment:
       The best practice for this should be @Test(expected= IllegalArgumentException.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] [incubator-dolphinscheduler] codecov-commenter commented on pull request #3746: [fix-3745][server] server get tasktype NPE exception

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


   # [Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/3746?src=pr&el=h1) Report
   > Merging [#3746](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/3746?src=pr&el=desc) into [dev](https://codecov.io/gh/apache/incubator-dolphinscheduler/commit/3942740941533721694ceef2177c5363b0d0f200?el=desc) will **decrease** coverage by `0.05%`.
   > The diff coverage is `67.85%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/3746/graphs/tree.svg?width=650&height=150&src=pr&token=bv9iXXRLi9)](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/3746?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##                dev    #3746      +/-   ##
   ============================================
   - Coverage     39.98%   39.92%   -0.06%     
   + Complexity     2901     2897       -4     
   ============================================
     Files           457      457              
     Lines         21665    21672       +7     
     Branches       2637     2641       +4     
   ============================================
   - Hits           8662     8653       -9     
   - Misses        12185    12201      +16     
     Partials        818      818              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/3746?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...lphinscheduler/server/worker/task/TaskManager.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/3746/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci90YXNrL1Rhc2tNYW5hZ2VyLmphdmE=) | `65.00% <55.55%> (+7.85%)` | `9.00 <9.00> (+1.00)` | |
   | [...hinscheduler/common/utils/TaskParametersUtils.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/3746/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL3V0aWxzL1Rhc2tQYXJhbWV0ZXJzVXRpbHMuamF2YQ==) | `68.18% <73.68%> (+1.51%)` | `12.00 <11.00> (+1.00)` | |
   | [...er/master/processor/queue/TaskResponseService.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/3746/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9wcm9jZXNzb3IvcXVldWUvVGFza1Jlc3BvbnNlU2VydmljZS5qYXZh) | `33.33% <0.00%> (-21.57%)` | `5.00% <0.00%> (-1.00%)` | |
   | [...rver/master/processor/queue/TaskResponseEvent.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/3746/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9wcm9jZXNzb3IvcXVldWUvVGFza1Jlc3BvbnNlRXZlbnQuamF2YQ==) | `49.01% <0.00%> (-9.81%)` | `9.00% <0.00%> (-5.00%)` | |
   | [...inscheduler/service/zk/CuratorZookeeperClient.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/3746/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3NlcnZpY2UvemsvQ3VyYXRvclpvb2tlZXBlckNsaWVudC5qYXZh) | `62.16% <0.00%> (+2.70%)` | `7.00% <0.00%> (ø%)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/3746?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/3746?src=pr&el=footer). Last update [3942740...30f8f27](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/3746?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] [incubator-dolphinscheduler] Technoboy- commented on a change in pull request #3746: [fix-3745][server] server get tasktype NPE exception

Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #3746:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3746#discussion_r489159845



##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/TaskParametersUtils.java
##########
@@ -55,41 +55,42 @@ private TaskParametersUtils() {
      * @return task parameters
      */
     public static AbstractParameters getParameters(String taskType, String parameter) {
-        try {
-            switch (EnumUtils.getEnum(TaskType.class, taskType)) {
-                case SUB_PROCESS:
-                    return JSONUtils.parseObject(parameter, SubProcessParameters.class);
-                case SHELL:
-                case WATERDROP:
-                    return JSONUtils.parseObject(parameter, ShellParameters.class);
-                case PROCEDURE:
-                    return JSONUtils.parseObject(parameter, ProcedureParameters.class);
-                case SQL:
-                    return JSONUtils.parseObject(parameter, SqlParameters.class);
-                case MR:
-                    return JSONUtils.parseObject(parameter, MapreduceParameters.class);
-                case SPARK:
-                    return JSONUtils.parseObject(parameter, SparkParameters.class);
-                case PYTHON:
-                    return JSONUtils.parseObject(parameter, PythonParameters.class);
-                case DEPENDENT:
-                    return JSONUtils.parseObject(parameter, DependentParameters.class);
-                case FLINK:
-                    return JSONUtils.parseObject(parameter, FlinkParameters.class);
-                case HTTP:
-                    return JSONUtils.parseObject(parameter, HttpParameters.class);
-                case DATAX:
-                    return JSONUtils.parseObject(parameter, DataxParameters.class);
-                case CONDITIONS:
-                    return JSONUtils.parseObject(parameter, ConditionsParameters.class);
-                case SQOOP:
-                    return JSONUtils.parseObject(parameter, SqoopParameters.class);
-                default:
-                    return null;
-            }
-        } catch (Exception e) {
-            logger.error(e.getMessage(), e);
+        TaskType anEnum = EnumUtils.getEnum(TaskType.class, taskType);
+        if (anEnum == null) {
+            logger.error("not support task type: {}", taskType);
+            return null;

Review comment:
       Why not throw InvalidArgumentException?




----------------------------------------------------------------
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] [incubator-dolphinscheduler] sonarcloud[bot] commented on pull request #3746: [fix-3745][server] server get tasktype NPE exception

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


   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=3746&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=3746&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3746&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=3746&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=3746&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3746&resolved=false&types=VULNERABILITY) (and [<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/issues?id=apache-dolphinscheduler&pullRequest=3746&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/proje
 ct/issues?id=apache-dolphinscheduler&pullRequest=3746&resolved=false&types=SECURITY_HOTSPOT) to review)  
   [<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=3746&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=3746&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3746&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='76.7%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3746&metric=new_coverage&view=list) [76.7% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3746&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=3746&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3746&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] [incubator-dolphinscheduler] zhuangchong commented on a change in pull request #3746: [fix-3745][server] server get tasktype NPE exception

Posted by GitBox <gi...@apache.org>.
zhuangchong commented on a change in pull request #3746:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3746#discussion_r489300306



##########
File path: dolphinscheduler-server/src/test/java/org/apache/dolphinscheduler/server/worker/task/TaskManagerTest.java
##########
@@ -95,9 +97,19 @@ public void testNewTask() {
         Assert.assertNotNull(TaskManager.newTask(taskExecutionContext,taskLogger));
         taskExecutionContext.setTaskType("SQOOP");
         Assert.assertNotNull(TaskManager.newTask(taskExecutionContext,taskLogger));
-        //taskExecutionContext.setTaskType(null);
-        //Assert.assertNull(TaskManager.newTask(taskExecutionContext,taskLogger));
-        //taskExecutionContext.setTaskType("XXX");
-        //Assert.assertNotNull(TaskManager.newTask(taskExecutionContext,taskLogger));
+        try {
+            taskExecutionContext.setTaskType(null);
+            TaskManager.newTask(taskExecutionContext,taskLogger);
+        } catch (Exception e) {
+            logger.error(e.getMessage());
+        }
+
+        try {
+            taskExecutionContext.setTaskType("XXX");

Review comment:
       done

##########
File path: dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/TaskManager.java
##########
@@ -69,8 +74,9 @@ public static AbstractTask newTask(TaskExecutionContext taskExecutionContext, Lo
             case SQOOP:
                 return new SqoopTask(taskExecutionContext, logger);
             default:
-                logger.error("unsupport task type: {}", taskExecutionContext.getTaskType());
-                throw new IllegalArgumentException("not support task type");
+                String msg = String.format("not support task type: %s", taskExecutionContext.getTaskType());

Review comment:
       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] [incubator-dolphinscheduler] sonarcloud[bot] commented on pull request #3746: [fix-3745][server] server get tasktype NPE exception

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


   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=3746&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=3746&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3746&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=3746&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=3746&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3746&resolved=false&types=VULNERABILITY) (and [<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/issues?id=apache-dolphinscheduler&pullRequest=3746&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/proje
 ct/issues?id=apache-dolphinscheduler&pullRequest=3746&resolved=false&types=SECURITY_HOTSPOT) to review)  
   [<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=3746&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=3746&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3746&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='80.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3746&metric=new_coverage&view=list) [80.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3746&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=3746&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3746&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