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/01/05 04:11:19 UTC

[GitHub] [incubator-dolphinscheduler] deanwong opened a new pull request #4372: [DS-130][feat] pass global param values when starting new process instance

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


   ## What is the purpose of the pull request
   
   Could pass global param values when starting a new process instance.
   The main idea is to merge the start param values into the new process instance's global params.
   
   ## Brief changelog
   
      - add optional param for start-process-instance api
      - reuse command_param in command table for persistence
      - overload curingGlobalParams function in ParameterUtils
      - not adapt the UI code yet
   
   ## Verify this pull request
   
   This change added tests and can be verified as follows:
   
     -  Added test case in ParameterUitlsTest
     -  Added test case in ProcessServiceTest to verify the start param value could be  merged into new process instance's global params
     - Manually verified the start-process-instance API by testing locally
   
   The test process definition json as below 
   `{"globalParams":[{"prop":"tenant_id","direct":"IN","type":"VARCHAR","value":""}],"tasks":[{"type":"SHELL","id":"tasks-62232","name":"echo","params":{"resourceList":[],"localParams":[],"rawScript":"echo ${tenant_id}"},"description":"","runFlag":"NORMAL","conditionResult":{"successNode":[""],"failedNode":[""]},"dependence":{},"maxRetryTimes":"0","retryInterval":"1","delayTime":"0","timeout":{"strategy":"","interval":null,"enable":false},"waitStartTimeout":{},"taskInstancePriority":"MEDIUM","workerGroup":"default","preTasks":[]}],"tenantId":2,"timeout":0}`
   ![ds-pull-01](https://user-images.githubusercontent.com/4144156/103604390-e8b13080-4f4b-11eb-8d4c-eac9eac311c1.png)
   ![ds-pull-02](https://user-images.githubusercontent.com/4144156/103604442-0ed6d080-4f4c-11eb-8adc-7762654fee5d.png)
   ![ds-pull-03](https://user-images.githubusercontent.com/4144156/103605119-01bae100-4f4e-11eb-8f82-b0ceb91c95d9.png)
   
   One question here?
   Do I need to modify the ProcessInstanceJson as well? But I did not find anything wrong even add multiple tasks with global param. Please give me some suggestions.
   


----------------------------------------------------------------
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 #4372: [Feature-#130] pass global param values when starting new process instance

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


   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=4372&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=4372&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='58.3%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&metric=new_coverage&view=list) [58.3% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&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=4372&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&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] CalvinKirs commented on a change in pull request #4372: [Feature-#130] pass global param values when starting new process instance

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



##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/ParameterUtils.java
##########
@@ -149,6 +149,33 @@ public static void setInParameter(int index, PreparedStatement stmt, DataType da
         }
     }
 
+    /**
+     * curing user define parameters
+     *
+     * @param globalParamMap  global param map
+     * @param globalParamList global param list
+     * @param commandType     command type
+     * @param scheduleTime    schedule time
+     * @param startParams     start param map
+     * @return curing user define parameters
+     */
+    public static String curingGlobalParams(Map<String, String> globalParamMap, List<Property> globalParamList,
+                                            CommandType commandType, Date scheduleTime, Map<String, String> startParams) {
+        String globalParamsJson = curingGlobalParams(globalParamMap, globalParamList, commandType, scheduleTime);
+        // merge start params
+        if (startParams != null && startParams.size() > 0) {

Review comment:
       If the parameters you set at startup are like this (key->time,value ->$[yyyy-mm-dd]) , it may eventually lead to unresolved




----------------------------------------------------------------
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] CalvinKirs commented on a change in pull request #4372: [Feature-#130] pass global param values when starting new process instance

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



##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/ParameterUtils.java
##########
@@ -149,6 +149,33 @@ public static void setInParameter(int index, PreparedStatement stmt, DataType da
         }
     }
 
+    /**
+     * curing user define parameters
+     *
+     * @param globalParamMap  global param map
+     * @param globalParamList global param list
+     * @param commandType     command type
+     * @param scheduleTime    schedule time
+     * @param startParams     start param map
+     * @return curing user define parameters
+     */
+    public static String curingGlobalParams(Map<String, String> globalParamMap, List<Property> globalParamList,
+                                            CommandType commandType, Date scheduleTime, Map<String, String> startParams) {
+        String globalParamsJson = curingGlobalParams(globalParamMap, globalParamList, commandType, scheduleTime);
+        // merge start params
+        if (startParams != null && startParams.size() > 0) {

Review comment:
       I think there may be a problem here, if the value of the parameter set at startup
   Is the reference type, eg: (k->time,v->$[yyyy-mm-dd])




----------------------------------------------------------------
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 #4372: [DS-130][feat] pass global param values when starting new process instance

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


   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=4372&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=4372&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='63.9%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&metric=new_coverage&view=list) [63.9% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&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=4372&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&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] deanwong commented on a change in pull request #4372: [Feature-#130] pass global param values when starting new process instance

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



##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/ParameterUtils.java
##########
@@ -149,6 +149,33 @@ public static void setInParameter(int index, PreparedStatement stmt, DataType da
         }
     }
 
+    /**
+     * curing user define parameters
+     *
+     * @param globalParamMap  global param map
+     * @param globalParamList global param list
+     * @param commandType     command type
+     * @param scheduleTime    schedule time
+     * @param startParams     start param map
+     * @return curing user define parameters
+     */
+    public static String curingGlobalParams(Map<String, String> globalParamMap, List<Property> globalParamList,
+                                            CommandType commandType, Date scheduleTime, Map<String, String> startParams) {
+        String globalParamsJson = curingGlobalParams(globalParamMap, globalParamList, commandType, scheduleTime);
+        // merge start params
+        if (startParams != null && startParams.size() > 0) {

Review comment:
       I think the startup params should be fixed values. If the UI exists, the popup will let user input fixed value, and the fixed value pass into global param. User can set variables in global params, but why does he set the variables in start params?




----------------------------------------------------------------
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] CalvinKirs merged pull request #4372: [Feature-#130] pass global param values when starting new process instance

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


   


----------------------------------------------------------------
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 #4372: [DS-130][feat] pass global param values when starting new process instance

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


   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=4372&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=4372&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='63.9%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&metric=new_coverage&view=list) [63.9% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&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=4372&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&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] CalvinKirs commented on a change in pull request #4372: [Feature-#130] pass global param values when starting new process instance

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



##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/ParameterUtils.java
##########
@@ -149,6 +149,33 @@ public static void setInParameter(int index, PreparedStatement stmt, DataType da
         }
     }
 
+    /**
+     * curing user define parameters
+     *
+     * @param globalParamMap  global param map
+     * @param globalParamList global param list
+     * @param commandType     command type
+     * @param scheduleTime    schedule time
+     * @param startParams     start param map
+     * @return curing user define parameters
+     */
+    public static String curingGlobalParams(Map<String, String> globalParamMap, List<Property> globalParamList,
+                                            CommandType commandType, Date scheduleTime, Map<String, String> startParams) {
+        String globalParamsJson = curingGlobalParams(globalParamMap, globalParamList, commandType, scheduleTime);
+        // merge start params
+        if (startParams != null && startParams.size() > 0) {

Review comment:
       Yes, but in fact, we do use this in some cases, so we need to consider such a situation ((key->time,value ->$[yyyy-mm-dd]),The user used such an expression instead of a fixed value)




----------------------------------------------------------------
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] CalvinKirs commented on pull request #4372: [Feature-#130] pass global param values when starting new process instance

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


   considering that it's your first contribution, I think we can get deep communiction, you can contact me through mail or add wechat(Kris_Evil), when mail or added, please tell me who you are, I think I can help to familiar with the DolphinScheduler if you meet with problems.


----------------------------------------------------------------
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-io edited a comment on pull request #4372: [Feature-#130] pass global param values when starting new process instance

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #4372:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4372#issuecomment-754391326


   # [Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372?src=pr&el=h1) Report
   > Merging [#4372](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372?src=pr&el=desc) (c48bb70) into [dev](https://codecov.io/gh/apache/incubator-dolphinscheduler/commit/0b71c77b2d2ff85afe2b440e53773fe2c428bed3?el=desc) (0b71c77) will **increase** coverage by `0.25%`.
   > The diff coverage is `43.75%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/graphs/tree.svg?width=650&height=150&src=pr&token=bv9iXXRLi9)](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##                dev    #4372      +/-   ##
   ============================================
   + Coverage     43.22%   43.48%   +0.25%     
   - Complexity     3183     3204      +21     
   ============================================
     Files           471      471              
     Lines         21728    21812      +84     
     Branches       2626     2646      +20     
   ============================================
   + Hits           9392     9484      +92     
   + Misses        11440    11421      -19     
   - Partials        896      907      +11     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...inscheduler/api/controller/ExecutorController.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvRXhlY3V0b3JDb250cm9sbGVyLmphdmE=) | `9.52% <0.00%> (-1.59%)` | `2.00 <0.00> (ø)` | |
   | [.../dolphinscheduler/api/service/ExecutorService.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvRXhlY3V0b3JTZXJ2aWNlLmphdmE=) | `34.46% <0.00%> (-0.30%)` | `20.00 <0.00> (ø)` | |
   | [.../org/apache/dolphinscheduler/common/Constants.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL0NvbnN0YW50cy5qYXZh) | `85.00% <ø> (ø)` | `1.00 <0.00> (ø)` | |
   | [...lphinscheduler/service/process/ProcessService.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3NlcnZpY2UvcHJvY2Vzcy9Qcm9jZXNzU2VydmljZS5qYXZh) | `31.77% <63.63%> (+6.07%)` | `54.00 <0.00> (+8.00)` | |
   | [...heduler/api/controller/TaskInstanceController.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvVGFza0luc3RhbmNlQ29udHJvbGxlci5qYXZh) | `82.35% <0.00%> (-17.65%)` | `3.00% <0.00%> (ø%)` | |
   | [...er/master/dispatch/host/assign/RandomSelector.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9kaXNwYXRjaC9ob3N0L2Fzc2lnbi9SYW5kb21TZWxlY3Rvci5qYXZh) | `77.77% <0.00%> (-5.56%)` | `3.00% <0.00%> (-1.00%)` | |
   | [...er/api/controller/ProcessDefinitionController.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvUHJvY2Vzc0RlZmluaXRpb25Db250cm9sbGVyLmphdmE=) | `74.74% <0.00%> (-1.55%)` | `20.00% <0.00%> (ø%)` | |
   | [...uler/api/service/impl/DataAnalysisServiceImpl.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvaW1wbC9EYXRhQW5hbHlzaXNTZXJ2aWNlSW1wbC5qYXZh) | `98.91% <0.00%> (-1.09%)` | `30.00% <0.00%> (+1.00%)` | :arrow_down: |
   | [.../org/apache/dolphinscheduler/api/enums/Status.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2VudW1zL1N0YXR1cy5qYXZh) | `100.00% <0.00%> (ø)` | `5.00% <0.00%> (ø%)` | |
   | [...heduler/server/master/runner/MasterExecThread.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9ydW5uZXIvTWFzdGVyRXhlY1RocmVhZC5qYXZh) | `0.00% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
   | ... and [8 more](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372?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/4372?src=pr&el=footer). Last update [0b71c77...c48bb70](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372?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] chengshiwen commented on a change in pull request #4372: [Feature-#130] pass global param values when starting new process instance

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



##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ExecutorController.java
##########
@@ -107,7 +108,8 @@ public Result startProcessInstance(@ApiIgnore @RequestAttribute(value = Constant
                                        @RequestParam(value = "runMode", required = false) RunMode runMode,
                                        @RequestParam(value = "processInstancePriority", required = false) Priority processInstancePriority,
                                        @RequestParam(value = "workerGroup", required = false, defaultValue = "default") String workerGroup,
-                                       @RequestParam(value = "timeout", required = false) Integer timeout) throws ParseException {
+                                       @RequestParam(value = "timeout", required = false) Integer timeout,
+                                       @RequestParam(value = "startParams", required = false) String startParams) throws ParseException {
         logger.info("login user {}, start process instance, project name: {}, process definition id: {}, schedule time: {}, "
                         + "failure policy: {}, node name: {}, node dep: {}, notify type: {}, "
                         + "notify group id: {},receivers:{},receiversCc:{}, run mode: {},process instance priority:{}, workerGroup: {}, timeout: {}",

Review comment:
       It would be better to print the `startParams`




----------------------------------------------------------------
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] CalvinKirs commented on pull request #4372: [DS-130][feat] pass global param values when starting new process instance

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


   hi, deeply thanks for your contribution, is there any discussion about this design?


----------------------------------------------------------------
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] deanwong commented on a change in pull request #4372: [Feature-#130] pass global param values when starting new process instance

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



##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/ParameterUtils.java
##########
@@ -149,6 +149,33 @@ public static void setInParameter(int index, PreparedStatement stmt, DataType da
         }
     }
 
+    /**
+     * curing user define parameters
+     *
+     * @param globalParamMap  global param map
+     * @param globalParamList global param list
+     * @param commandType     command type
+     * @param scheduleTime    schedule time
+     * @param startParams     start param map
+     * @return curing user define parameters
+     */
+    public static String curingGlobalParams(Map<String, String> globalParamMap, List<Property> globalParamList,
+                                            CommandType commandType, Date scheduleTime, Map<String, String> startParams) {
+        String globalParamsJson = curingGlobalParams(globalParamMap, globalParamList, commandType, scheduleTime);
+        // merge start params
+        if (startParams != null && startParams.size() > 0) {

Review comment:
       I've got your point, user may pass {'biz_date','$[add_months(yyyyMMdd,12*N)]'} to process instance, so the $[add_months(yyyyMMdd,12*N)] should be replaced before merging into intance global params. I will figure out the solution.  I did not know the feature of 'time custom parameter' yesterday, sorry for that.  




----------------------------------------------------------------
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] deanwong removed a comment on pull request #4372: [Feature-#130] pass global param values when starting new process instance

Posted by GitBox <gi...@apache.org>.
deanwong removed a comment on pull request #4372:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4372#issuecomment-757802437


   Something wrong at the building phase. Can anyone please re-trigger it?
   ![image](https://user-images.githubusercontent.com/4144156/104164633-296ce600-5433-11eb-8c67-c406bacc48e6.png)
   


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

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

[GitHub] [incubator-dolphinscheduler] CalvinKirs commented on a change in pull request #4372: [Feature-#130] pass global param values when starting new process instance

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



##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/ParameterUtils.java
##########
@@ -149,6 +149,33 @@ public static void setInParameter(int index, PreparedStatement stmt, DataType da
         }
     }
 
+    /**
+     * curing user define parameters
+     *
+     * @param globalParamMap  global param map
+     * @param globalParamList global param list
+     * @param commandType     command type
+     * @param scheduleTime    schedule time
+     * @param startParams     start param map
+     * @return curing user define parameters
+     */
+    public static String curingGlobalParams(Map<String, String> globalParamMap, List<Property> globalParamList,
+                                            CommandType commandType, Date scheduleTime, Map<String, String> startParams) {
+        String globalParamsJson = curingGlobalParams(globalParamMap, globalParamList, commandType, scheduleTime);
+        // merge start params
+        if (startParams != null && startParams.size() > 0) {

Review comment:
       this is indeed a usage, we always want to be more flexible.If this situation is not considered, then problems may occur in the follow-up, which will be a bad experience for users.




----------------------------------------------------------------
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] CalvinKirs commented on pull request #4372: [Feature-#130] pass global param values when starting new process instance

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


   @CalvinKirs Serial parallel complement for testing


----------------------------------------------------------------
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] deanwong commented on pull request #4372: [DS-130][feat] pass global param values when starting new process instance

Posted by GitBox <gi...@apache.org>.
deanwong commented on pull request #4372:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4372#issuecomment-754424982


   @CalvinKirs  I saw the work plan https://github.com/apache/incubator-dolphinscheduler/projects/1#card-20657550


----------------------------------------------------------------
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 #4372: [Feature-#130] pass global param values when starting new process instance

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


   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=4372&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=4372&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='58.8%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&metric=new_coverage&view=list) [58.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&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=4372&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&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 #4372: [DS-130][feat] pass global param values when starting new process instance

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


   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=4372&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=4372&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='63.9%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&metric=new_coverage&view=list) [63.9% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&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=4372&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&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] deanwong commented on pull request #4372: [Feature-#130] pass global param values when starting new process instance

Posted by GitBox <gi...@apache.org>.
deanwong commented on pull request #4372:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4372#issuecomment-757802437


   Something wrong at the building phase. Can anyone please re-trigger it?
   ![image](https://user-images.githubusercontent.com/4144156/104164633-296ce600-5433-11eb-8c67-c406bacc48e6.png)
   


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

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

[GitHub] [incubator-dolphinscheduler] deanwong commented on a change in pull request #4372: [Feature-#130] pass global param values when starting new process instance

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



##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/ParameterUtils.java
##########
@@ -149,6 +149,33 @@ public static void setInParameter(int index, PreparedStatement stmt, DataType da
         }
     }
 
+    /**
+     * curing user define parameters
+     *
+     * @param globalParamMap  global param map
+     * @param globalParamList global param list
+     * @param commandType     command type
+     * @param scheduleTime    schedule time
+     * @param startParams     start param map
+     * @return curing user define parameters
+     */
+    public static String curingGlobalParams(Map<String, String> globalParamMap, List<Property> globalParamList,
+                                            CommandType commandType, Date scheduleTime, Map<String, String> startParams) {
+        String globalParamsJson = curingGlobalParams(globalParamMap, globalParamList, commandType, scheduleTime);
+        // merge start params
+        if (startParams != null && startParams.size() > 0) {

Review comment:
       @CalvinKirs I have changed some to support date/time expression. Please have a check.




----------------------------------------------------------------
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-io edited a comment on pull request #4372: [Feature-#130] pass global param values when starting new process instance

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #4372:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4372#issuecomment-754391326


   # [Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372?src=pr&el=h1) Report
   > Merging [#4372](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372?src=pr&el=desc) (12576f9) into [dev](https://codecov.io/gh/apache/incubator-dolphinscheduler/commit/0b71c77b2d2ff85afe2b440e53773fe2c428bed3?el=desc) (0b71c77) will **increase** coverage by `0.25%`.
   > The diff coverage is `43.75%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/graphs/tree.svg?width=650&height=150&src=pr&token=bv9iXXRLi9)](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##                dev    #4372      +/-   ##
   ============================================
   + Coverage     43.22%   43.48%   +0.25%     
   - Complexity     3183     3204      +21     
   ============================================
     Files           471      471              
     Lines         21728    21812      +84     
     Branches       2626     2646      +20     
   ============================================
   + Hits           9392     9484      +92     
   + Misses        11440    11421      -19     
   - Partials        896      907      +11     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...inscheduler/api/controller/ExecutorController.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvRXhlY3V0b3JDb250cm9sbGVyLmphdmE=) | `9.52% <0.00%> (-1.59%)` | `2.00 <0.00> (ø)` | |
   | [.../dolphinscheduler/api/service/ExecutorService.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvRXhlY3V0b3JTZXJ2aWNlLmphdmE=) | `34.46% <0.00%> (-0.30%)` | `20.00 <0.00> (ø)` | |
   | [.../org/apache/dolphinscheduler/common/Constants.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL0NvbnN0YW50cy5qYXZh) | `85.00% <ø> (ø)` | `1.00 <0.00> (ø)` | |
   | [...lphinscheduler/service/process/ProcessService.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3NlcnZpY2UvcHJvY2Vzcy9Qcm9jZXNzU2VydmljZS5qYXZh) | `31.77% <63.63%> (+6.07%)` | `54.00 <0.00> (+8.00)` | |
   | [...heduler/api/controller/TaskInstanceController.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvVGFza0luc3RhbmNlQ29udHJvbGxlci5qYXZh) | `82.35% <0.00%> (-17.65%)` | `3.00% <0.00%> (ø%)` | |
   | [...er/master/dispatch/host/assign/RandomSelector.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9kaXNwYXRjaC9ob3N0L2Fzc2lnbi9SYW5kb21TZWxlY3Rvci5qYXZh) | `77.77% <0.00%> (-5.56%)` | `3.00% <0.00%> (-1.00%)` | |
   | [...er/api/controller/ProcessDefinitionController.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvUHJvY2Vzc0RlZmluaXRpb25Db250cm9sbGVyLmphdmE=) | `74.74% <0.00%> (-1.55%)` | `20.00% <0.00%> (ø%)` | |
   | [...uler/api/service/impl/DataAnalysisServiceImpl.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvaW1wbC9EYXRhQW5hbHlzaXNTZXJ2aWNlSW1wbC5qYXZh) | `98.91% <0.00%> (-1.09%)` | `30.00% <0.00%> (+1.00%)` | :arrow_down: |
   | [.../org/apache/dolphinscheduler/api/enums/Status.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2VudW1zL1N0YXR1cy5qYXZh) | `100.00% <0.00%> (ø)` | `5.00% <0.00%> (ø%)` | |
   | [...heduler/server/master/runner/MasterExecThread.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9ydW5uZXIvTWFzdGVyRXhlY1RocmVhZC5qYXZh) | `0.00% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
   | ... and [8 more](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372?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/4372?src=pr&el=footer). Last update [0b71c77...c48bb70](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372?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] CalvinKirs commented on a change in pull request #4372: [Feature-#130] pass global param values when starting new process instance

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



##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/ParameterUtils.java
##########
@@ -149,6 +149,33 @@ public static void setInParameter(int index, PreparedStatement stmt, DataType da
         }
     }
 
+    /**
+     * curing user define parameters
+     *
+     * @param globalParamMap  global param map
+     * @param globalParamList global param list
+     * @param commandType     command type
+     * @param scheduleTime    schedule time
+     * @param startParams     start param map
+     * @return curing user define parameters
+     */
+    public static String curingGlobalParams(Map<String, String> globalParamMap, List<Property> globalParamList,
+                                            CommandType commandType, Date scheduleTime, Map<String, String> startParams) {
+        String globalParamsJson = curingGlobalParams(globalParamMap, globalParamList, commandType, scheduleTime);
+        // merge start params
+        if (startParams != null && startParams.size() > 0) {

Review comment:
       this is indeed a usage, we always want to be more flexible.




----------------------------------------------------------------
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] deanwong commented on a change in pull request #4372: [Feature-#130] pass global param values when starting new process instance

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



##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/ParameterUtils.java
##########
@@ -149,6 +149,33 @@ public static void setInParameter(int index, PreparedStatement stmt, DataType da
         }
     }
 
+    /**
+     * curing user define parameters
+     *
+     * @param globalParamMap  global param map
+     * @param globalParamList global param list
+     * @param commandType     command type
+     * @param scheduleTime    schedule time
+     * @param startParams     start param map
+     * @return curing user define parameters
+     */
+    public static String curingGlobalParams(Map<String, String> globalParamMap, List<Property> globalParamList,
+                                            CommandType commandType, Date scheduleTime, Map<String, String> startParams) {
+        String globalParamsJson = curingGlobalParams(globalParamMap, globalParamList, commandType, scheduleTime);
+        // merge start params
+        if (startParams != null && startParams.size() > 0) {

Review comment:
       Thanks for your comment, I don't understand the reference types, would you please show me a JSON sample (maybe global params) which contains this type.




----------------------------------------------------------------
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 #4372: [Feature-#130] pass global param values when starting new process instance

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


   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=4372&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=4372&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='58.8%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&metric=new_coverage&view=list) [58.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&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=4372&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&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 #4372: [Feature-#130] pass global param values when starting new process instance

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


   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=4372&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=4372&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='58.8%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&metric=new_coverage&view=list) [58.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&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=4372&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&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] deanwong commented on a change in pull request #4372: [Feature-#130] pass global param values when starting new process instance

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



##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ExecutorController.java
##########
@@ -107,7 +108,8 @@ public Result startProcessInstance(@ApiIgnore @RequestAttribute(value = Constant
                                        @RequestParam(value = "runMode", required = false) RunMode runMode,
                                        @RequestParam(value = "processInstancePriority", required = false) Priority processInstancePriority,
                                        @RequestParam(value = "workerGroup", required = false, defaultValue = "default") String workerGroup,
-                                       @RequestParam(value = "timeout", required = false) Integer timeout) throws ParseException {
+                                       @RequestParam(value = "timeout", required = false) Integer timeout,
+                                       @RequestParam(value = "startParams", required = false) String startParams) throws ParseException {
         logger.info("login user {}, start process instance, project name: {}, process definition id: {}, schedule time: {}, "
                         + "failure policy: {}, node name: {}, node dep: {}, notify type: {}, "
                         + "notify group id: {},receivers:{},receiversCc:{}, run mode: {},process instance priority:{}, workerGroup: {}, timeout: {}",

Review comment:
       Thanks for your suggestion, already added




----------------------------------------------------------------
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] deanwong commented on a change in pull request #4372: [Feature-#130] pass global param values when starting new process instance

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



##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/ParameterUtils.java
##########
@@ -149,6 +149,33 @@ public static void setInParameter(int index, PreparedStatement stmt, DataType da
         }
     }
 
+    /**
+     * curing user define parameters
+     *
+     * @param globalParamMap  global param map
+     * @param globalParamList global param list
+     * @param commandType     command type
+     * @param scheduleTime    schedule time
+     * @param startParams     start param map
+     * @return curing user define parameters
+     */
+    public static String curingGlobalParams(Map<String, String> globalParamMap, List<Property> globalParamList,
+                                            CommandType commandType, Date scheduleTime, Map<String, String> startParams) {
+        String globalParamsJson = curingGlobalParams(globalParamMap, globalParamList, commandType, scheduleTime);
+        // merge start params
+        if (startParams != null && startParams.size() > 0) {

Review comment:
       The popup will show when user click start one process.




----------------------------------------------------------------
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] deanwong commented on a change in pull request #4372: [Feature-#130] pass global param values when starting new process instance

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



##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/ParameterUtils.java
##########
@@ -149,6 +149,33 @@ public static void setInParameter(int index, PreparedStatement stmt, DataType da
         }
     }
 
+    /**
+     * curing user define parameters
+     *
+     * @param globalParamMap  global param map
+     * @param globalParamList global param list
+     * @param commandType     command type
+     * @param scheduleTime    schedule time
+     * @param startParams     start param map
+     * @return curing user define parameters
+     */
+    public static String curingGlobalParams(Map<String, String> globalParamMap, List<Property> globalParamList,
+                                            CommandType commandType, Date scheduleTime, Map<String, String> startParams) {
+        String globalParamsJson = curingGlobalParams(globalParamMap, globalParamList, commandType, scheduleTime);
+        // merge start params
+        if (startParams != null && startParams.size() > 0) {

Review comment:
       You mean the date value? Like {”time”:”2020-01-04”}, I think it should be treated as string value. If I have any misunderstood, please correct me. Thanks.




----------------------------------------------------------------
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 #4372: [Feature-#130] pass global param values when starting new process instance

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


   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=4372&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=4372&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='58.8%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&metric=new_coverage&view=list) [58.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&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=4372&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&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 #4372: [Feature-#130] pass global param values when starting new process instance

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


   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=4372&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=4372&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=4372&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=4372&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=4372&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4372&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='63.9%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&metric=new_coverage&view=list) [63.9% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&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=4372&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4372&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] codecov-io commented on pull request #4372: [DS-130][feat] pass global param values when starting new process instance

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


   # [Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372?src=pr&el=h1) Report
   > Merging [#4372](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372?src=pr&el=desc) (5b75eab) into [dev](https://codecov.io/gh/apache/incubator-dolphinscheduler/commit/0b71c77b2d2ff85afe2b440e53773fe2c428bed3?el=desc) (0b71c77) will **increase** coverage by `0.16%`.
   > The diff coverage is `60.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/graphs/tree.svg?width=650&height=150&src=pr&token=bv9iXXRLi9)](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##                dev    #4372      +/-   ##
   ============================================
   + Coverage     43.22%   43.39%   +0.16%     
   - Complexity     3183     3189       +6     
   ============================================
     Files           471      471              
     Lines         21728    21747      +19     
     Branches       2626     2632       +6     
   ============================================
   + Hits           9392     9437      +45     
   + Misses        11440    11402      -38     
   - Partials        896      908      +12     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...inscheduler/api/controller/ExecutorController.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvRXhlY3V0b3JDb250cm9sbGVyLmphdmE=) | `9.52% <0.00%> (-1.59%)` | `2.00 <0.00> (ø)` | |
   | [.../dolphinscheduler/api/service/ExecutorService.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvRXhlY3V0b3JTZXJ2aWNlLmphdmE=) | `34.46% <0.00%> (-0.30%)` | `20.00 <0.00> (ø)` | |
   | [.../org/apache/dolphinscheduler/common/Constants.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL0NvbnN0YW50cy5qYXZh) | `85.00% <ø> (ø)` | `1.00 <0.00> (ø)` | |
   | [.../dolphinscheduler/common/utils/ParameterUtils.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL3V0aWxzL1BhcmFtZXRlclV0aWxzLmphdmE=) | `46.61% <80.00%> (+3.09%)` | `19.00 <3.00> (+3.00)` | |
   | [...lphinscheduler/service/process/ProcessService.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3NlcnZpY2UvcHJvY2Vzcy9Qcm9jZXNzU2VydmljZS5qYXZh) | `31.52% <80.00%> (+5.82%)` | `53.00 <0.00> (+7.00)` | |
   | [...er/master/dispatch/host/assign/RandomSelector.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9kaXNwYXRjaC9ob3N0L2Fzc2lnbi9SYW5kb21TZWxlY3Rvci5qYXZh) | `77.77% <0.00%> (-5.56%)` | `3.00% <0.00%> (-1.00%)` | |
   | [...e/dolphinscheduler/remote/NettyRemotingClient.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL05ldHR5UmVtb3RpbmdDbGllbnQuamF2YQ==) | `50.00% <0.00%> (-2.78%)` | `9.00% <0.00%> (-2.00%)` | |
   | [...dolphinscheduler/remote/future/ResponseFuture.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL2Z1dHVyZS9SZXNwb25zZUZ1dHVyZS5qYXZh) | `81.35% <0.00%> (-1.70%)` | `18.00% <0.00%> (-1.00%)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372?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/4372?src=pr&el=footer). Last update [0b71c77...5b75eab](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4372?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] CalvinKirs commented on pull request #4372: [Feature-#130] pass global param values when starting new process instance

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


   Deeply thanks for your contribution. The setting at startup is a temporary setting parameter, so I don’t think we need to modify ProcessInstanceJson


----------------------------------------------------------------
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] CalvinKirs commented on a change in pull request #4372: [Feature-#130] pass global param values when starting new process instance

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



##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/ParameterUtils.java
##########
@@ -149,6 +149,33 @@ public static void setInParameter(int index, PreparedStatement stmt, DataType da
         }
     }
 
+    /**
+     * curing user define parameters
+     *
+     * @param globalParamMap  global param map
+     * @param globalParamList global param list
+     * @param commandType     command type
+     * @param scheduleTime    schedule time
+     * @param startParams     start param map
+     * @return curing user define parameters
+     */
+    public static String curingGlobalParams(Map<String, String> globalParamMap, List<Property> globalParamList,
+                                            CommandType commandType, Date scheduleTime, Map<String, String> startParams) {
+        String globalParamsJson = curingGlobalParams(globalParamMap, globalParamList, commandType, scheduleTime);
+        // merge start params
+        if (startParams != null && startParams.size() > 0) {

Review comment:
       I got it, but you have to consider this situation when you start setting parameters.




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