You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2022/11/25 03:30:12 UTC

[GitHub] [dolphinscheduler] insist777 opened a new pull request, #12990: [Improve][Api]Add WorkflowInstance openapi

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

   <!--Thanks very much for contributing to Apache DolphinScheduler. Please review https://dolphinscheduler.apache.org/en-us/community/development/pull-request.html before opening a pull request.-->
   
   ## Purpose of the pull request
   
   <!--(For example: This pull request adds checkstyle plugin).-->
   
   ## Brief change log
   
   <!--*(for example:)*
   - *Add maven-checkstyle-plugin to root pom.xml*
   -->
   
   ## Verify this pull request
   
   <!--*(Please pick either of the following options)*-->
   
   This pull request is code cleanup without any test coverage.
   
   *(or)*
   
   This pull request is already covered by existing tests, such as *(please describe tests)*.
   
   (or)
   
   This change added tests and can be verified as follows:
   
   <!--*(example:)*
   - *Added dolphinscheduler-dao tests for end-to-end.*
   - *Added CronUtilsTest to verify the change.*
   - *Manually verified the change by testing locally.* -->
   
   (or)
   
   If your pull request contain incompatible change, you should also add it to `docs/docs/en/guide/upgrede/incompatible.md`
   


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

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

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

[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #12990: [Feature][API] New restful API for workflowInstance

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


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/WorkflowInstanceV2Controller.java:
##########
@@ -0,0 +1,129 @@
+package org.apache.dolphinscheduler.api.controller;
+
+import static org.apache.dolphinscheduler.api.enums.Status.*;

Review Comment:
   > @SbloodyS Current regex `import\s+[^\*\s]+\*;(\r\n|\r|\n)` missed this case `import static xxx;\n`. I will submit a fix for this. : )
   
   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.

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

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

[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #12990: [Feature][API] New restful API for workflowInstance

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


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/WorkflowInstanceV2Controller.java:
##########
@@ -0,0 +1,129 @@
+package org.apache.dolphinscheduler.api.controller;
+
+import static org.apache.dolphinscheduler.api.enums.Status.*;

Review Comment:
   why this wildcard import still exists? cc @EricGao888 



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

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

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

[GitHub] [dolphinscheduler] codecov-commenter commented on pull request #12990: [Feature][API] New restful API for workflowInstance

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

   # [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/12990?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#12990](https://codecov.io/gh/apache/dolphinscheduler/pull/12990?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (dbd2c9c) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/b40c63f02da16a4e9d795cf016bf69f38754c25e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b40c63f) will **decrease** coverage by `0.03%`.
   > The diff coverage is `16.17%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##                dev   #12990      +/-   ##
   ============================================
   - Coverage     39.35%   39.31%   -0.04%     
   - Complexity     4270     4273       +3     
   ============================================
     Files          1067     1069       +2     
     Lines         40118    40156      +38     
     Branches       4606     4595      -11     
   ============================================
   - Hits          15790    15789       -1     
   - Misses        22552    22591      +39     
     Partials       1776     1776              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/12990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...workflowInstance/WorkflowInstanceQueryRequest.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2R0by93b3JrZmxvd0luc3RhbmNlL1dvcmtmbG93SW5zdGFuY2VRdWVyeVJlcXVlc3QuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...cheduler/api/service/impl/ExecutorServiceImpl.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvaW1wbC9FeGVjdXRvclNlcnZpY2VJbXBsLmphdmE=) | `43.40% <0.00%> (-0.34%)` | :arrow_down: |
   | [...r/api/service/impl/ProcessInstanceServiceImpl.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvaW1wbC9Qcm9jZXNzSW5zdGFuY2VTZXJ2aWNlSW1wbC5qYXZh) | `63.96% <0.00%> (-6.42%)` | :arrow_down: |
   | [...r/api/controller/WorkflowInstanceV2Controller.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvV29ya2Zsb3dJbnN0YW5jZVYyQ29udHJvbGxlci5qYXZh) | `84.61% <84.61%> (ø)` | |
   | [...er/master/dispatch/host/assign/RandomSelector.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9kaXNwYXRjaC9ob3N0L2Fzc2lnbi9SYW5kb21TZWxlY3Rvci5qYXZh) | `77.77% <0.00%> (-5.56%)` | :arrow_down: |
   | [...erver/master/processor/queue/TaskEventService.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9wcm9jZXNzb3IvcXVldWUvVGFza0V2ZW50U2VydmljZS5qYXZh) | `75.00% <0.00%> (-5.36%)` | :arrow_down: |
   | [...org/apache/dolphinscheduler/remote/utils/Host.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL3V0aWxzL0hvc3QuamF2YQ==) | `42.55% <0.00%> (-2.13%)` | :arrow_down: |
   | [...hinscheduler/plugin/alert/script/ScriptSender.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hbGVydC9kb2xwaGluc2NoZWR1bGVyLWFsZXJ0LXBsdWdpbnMvZG9scGhpbnNjaGVkdWxlci1hbGVydC1zY3JpcHQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcGx1Z2luL2FsZXJ0L3NjcmlwdC9TY3JpcHRTZW5kZXIuamF2YQ==) | `70.58% <0.00%> (-1.64%)` | :arrow_down: |
   | [...e/dolphinscheduler/remote/NettyRemotingClient.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL05ldHR5UmVtb3RpbmdDbGllbnQuamF2YQ==) | `51.38% <0.00%> (-1.39%)` | :arrow_down: |
   | [...cheduler/plugin/alert/dingtalk/DingTalkSender.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12990/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hbGVydC9kb2xwaGluc2NoZWR1bGVyLWFsZXJ0LXBsdWdpbnMvZG9scGhpbnNjaGVkdWxlci1hbGVydC1kaW5ndGFsay9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZG9scGhpbnNjaGVkdWxlci9wbHVnaW4vYWxlcnQvZGluZ3RhbGsvRGluZ1RhbGtTZW5kZXIuamF2YQ==) | `34.13% <0.00%> (-0.78%)` | :arrow_down: |
   | ... and [32 more](https://codecov.io/gh/apache/dolphinscheduler/pull/12990/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


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

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

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

[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #12990: [Feature][API] New restful API for workflowInstance

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

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12990)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=BUG) [![E](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/E-16px.png 'E')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=CODE_SMELL) [18 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=CODE_SMELL)
   
   [![15.5%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '15.5%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12990&metric=new_coverage&view=list) [15.5% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12990&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12990&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12990&metric=new_duplicated_lines_density&view=list)
   
   


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

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

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

[GitHub] [dolphinscheduler] insist777 commented on a diff in pull request #12990: [Feature][API] New restful API for workflowInstance

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


##########
dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/ProcessService.java:
##########
@@ -83,7 +83,7 @@ ProcessInstance handleCommand(String host,
 
     void removeTaskLogFile(Integer processInstanceId);
 
-    void deleteWorkTaskInstanceByProcessInstanceId(int processInstanceId);
+    void deleteWorkTaskInstanceByProcessInstanceId(ProcessInstance processInstance);

Review Comment:
   done.



##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessInstanceServiceImpl.java:
##########
@@ -766,6 +811,22 @@ public Map<String, Object> deleteProcessInstanceById(User loginUser, long projec
         return result;
     }
 
+    /**
+     * delete workflow instance by id, at the same time，delete task instance and their mapping relation data
+     *
+     * @param loginUser login user
+     * @param workflowInstanceId workflow instance id
+     * @return delete result code
+     */
+    @Override
+    public Map<String, Object> deleteProcessInstanceById(User loginUser, Integer workflowInstanceId) {
+        ProcessInstance processInstance = processInstanceMapper.selectById(workflowInstanceId);
+        ProcessDefinition processDefinition =

Review Comment:
   done.



##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessInstanceServiceImpl.java:
##########
@@ -344,6 +360,35 @@ public Result queryProcessInstanceList(User loginUser, long projectCode, long pr
         return result;
     }
 
+    /**
+     * paging query process instance list, filtering according to project, process definition, time range, keyword, process status
+     *
+     * @param loginUser login user
+     * @param projectName project name
+     * @param pageNo page number
+     * @param pageSize page size
+     * @param processDefineName process definition name
+     * @param searchVal search value
+     * @param stateType state type
+     * @param host host
+     * @param startDate start time
+     * @param endDate end time
+     * @return process instance list
+     */
+    @Override
+    public Result queryProcessInstanceList(User loginUser, String projectName, String processDefineName,
+                                           String startDate, String endDate, String searchVal, String executorName,
+                                           WorkflowExecutionStatus stateType, String host, Integer pageNo,
+                                           Integer pageSize) {
+        Project project = projectMapper.queryByName(projectName);
+        ProcessDefinition processDefinition =
+                processDefineMapper.queryByDefineName(project.getCode(), processDefineName);
+
+        return queryProcessInstanceList(loginUser, project.getCode(), processDefinition.getCode(),

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.

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

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

[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #12990: [Feature][API] New restful API for workflowInstance

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


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/dto/workflowInstance/WorkflowInstanceQueryRequest.java:
##########
@@ -0,0 +1,33 @@
+package org.apache.dolphinscheduler.api.dto.workflowInstance;
+
+import org.apache.dolphinscheduler.api.dto.PageQueryDto;
+import org.apache.dolphinscheduler.dao.entity.ProcessInstance;
+
+import lombok.Data;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import io.swagger.v3.oas.annotations.media.Schema;
+
+/**
+ * workflow instance request
+ */
+@JsonIgnoreProperties(ignoreUnknown = true)
+@JsonInclude(JsonInclude.Include.NON_NULL)
+@Data
+public class WorkflowInstanceQueryRequest extends PageQueryDto {
+
+    @Schema(name = "projectName", example = "PROJECT-NAME")
+    String projectName;
+
+    @Schema(name = "workflowName", example = "WORKFLOW-NAME")

Review Comment:
   generally LGTM, but as I know we also have filter base on `start date` `end date` `state` `host` and others, should we also add them within this patch?



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

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

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

[GitHub] [dolphinscheduler] SbloodyS commented on pull request #12990: [Feature][API] New restful API for workflowInstance

Posted by GitBox <gi...@apache.org>.
SbloodyS commented on PR #12990:
URL: https://github.com/apache/dolphinscheduler/pull/12990#issuecomment-1327496545

   > may i ask if we need to modify the UI at the same time?
   
   No. We are just refactoring the backend api in #10257. And the UI part will be implemented later.


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

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

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

[GitHub] [dolphinscheduler] DarkAssassinator commented on pull request #12990: [Feature][API] New restful API for workflowInstance

Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on PR #12990:
URL: https://github.com/apache/dolphinscheduler/pull/12990#issuecomment-1327341826

   may i ask if we need to modify the UI at the same time?


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

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

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

[GitHub] [dolphinscheduler] EricGao888 commented on a diff in pull request #12990: [Feature][API] New restful API for workflowInstance

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


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/WorkflowInstanceV2Controller.java:
##########
@@ -0,0 +1,129 @@
+package org.apache.dolphinscheduler.api.controller;
+
+import static org.apache.dolphinscheduler.api.enums.Status.*;

Review Comment:
   @SbloodyS Current regex `import\s+[^\*\s]+\*;(\r\n|\r|\n)` missed this case `import static xxx;\n`. I will submit a fix for this. : )



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

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

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

[GitHub] [dolphinscheduler] zhongjiajie merged pull request #12990: [Feature][API] New restful API for workflowInstance

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


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

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

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

[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #12990: [Feature][API] New restful API for workflowInstance

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


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessInstanceServiceImpl.java:
##########
@@ -344,6 +360,35 @@ public Result queryProcessInstanceList(User loginUser, long projectCode, long pr
         return result;
     }
 
+    /**
+     * paging query process instance list, filtering according to project, process definition, time range, keyword, process status
+     *
+     * @param loginUser login user
+     * @param projectName project name
+     * @param pageNo page number
+     * @param pageSize page size
+     * @param processDefineName process definition name
+     * @param searchVal search value
+     * @param stateType state type
+     * @param host host
+     * @param startDate start time
+     * @param endDate end time
+     * @return process instance list
+     */
+    @Override
+    public Result queryProcessInstanceList(User loginUser, String projectName, String processDefineName,
+                                           String startDate, String endDate, String searchVal, String executorName,
+                                           WorkflowExecutionStatus stateType, String host, Integer pageNo,
+                                           Integer pageSize) {
+        Project project = projectMapper.queryByName(projectName);
+        ProcessDefinition processDefinition =
+                processDefineMapper.queryByDefineName(project.getCode(), processDefineName);
+
+        return queryProcessInstanceList(loginUser, project.getCode(), processDefinition.getCode(),

Review Comment:
   I do not think reusing the old method is a good idea, we should better query by `ProcessInstance` entity instead of reusing the old one, which you can find more detail in https://github.com/zhongjiajie/dolphinscheduler/blob/7d0e2cbbb9fcd2e1543f5166f0518eda03afae89/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java#L632-L633



##########
dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/ProcessService.java:
##########
@@ -83,7 +83,7 @@ ProcessInstance handleCommand(String host,
 
     void removeTaskLogFile(Integer processInstanceId);
 
-    void deleteWorkTaskInstanceByProcessInstanceId(int processInstanceId);
+    void deleteWorkTaskInstanceByProcessInstanceId(ProcessInstance processInstance);

Review Comment:
   should we also change this name? we not delete by id anymore



##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/dto/workflowInstance/WorkflowInstanceListPagingResponse.java:
##########
@@ -0,0 +1,20 @@
+package org.apache.dolphinscheduler.api.dto.workflowInstance;
+
+import org.apache.dolphinscheduler.api.utils.PageInfo;
+import org.apache.dolphinscheduler.api.utils.Result;
+import org.apache.dolphinscheduler.dao.entity.ProcessInstance;
+
+/**
+ * workflow instance list paging response
+ */
+public class WorkflowInstanceListPagingResponse extends Result {

Review Comment:
   we should directly use class Result instead of creating the new one, https://github.com/apache/dolphinscheduler/blob/af9374c2c201763ce4302f761389de18f24aa410/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/WorkflowV2Controller.java#L120-L126



##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessInstanceServiceImpl.java:
##########
@@ -766,6 +811,22 @@ public Map<String, Object> deleteProcessInstanceById(User loginUser, long projec
         return result;
     }
 
+    /**
+     * delete workflow instance by id, at the same time，delete task instance and their mapping relation data
+     *
+     * @param loginUser login user
+     * @param workflowInstanceId workflow instance id
+     * @return delete result code
+     */
+    @Override
+    public Map<String, Object> deleteProcessInstanceById(User loginUser, Integer workflowInstanceId) {
+        ProcessInstance processInstance = processInstanceMapper.selectById(workflowInstanceId);
+        ProcessDefinition processDefinition =

Review Comment:
   should we add an NPE catch in this object?



##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/dto/workflowInstance/WorkflowInstanceQueryRequest.java:
##########
@@ -0,0 +1,47 @@
+package org.apache.dolphinscheduler.api.dto.workflowInstance;
+
+import org.apache.dolphinscheduler.api.dto.PageQueryDto;
+import org.apache.dolphinscheduler.common.enums.TaskExecuteType;
+import org.apache.dolphinscheduler.common.enums.WorkflowExecutionStatus;
+
+import lombok.Data;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import io.swagger.v3.oas.annotations.media.Schema;
+
+/**
+ * workflow instance request
+ */
+@JsonIgnoreProperties(ignoreUnknown = true)
+@JsonInclude(JsonInclude.Include.NON_NULL)
+@Data
+public class WorkflowInstanceQueryRequest extends PageQueryDto {

Review Comment:
   It should more likely to entity `ProcessInstance` instead of the old search param from the old one



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

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

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

[GitHub] [dolphinscheduler] insist777 commented on a diff in pull request #12990: [Feature][API] New restful API for workflowInstance

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


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/dto/workflowInstance/WorkflowInstanceListPagingResponse.java:
##########
@@ -0,0 +1,20 @@
+package org.apache.dolphinscheduler.api.dto.workflowInstance;
+
+import org.apache.dolphinscheduler.api.utils.PageInfo;
+import org.apache.dolphinscheduler.api.utils.Result;
+import org.apache.dolphinscheduler.dao.entity.ProcessInstance;
+
+/**
+ * workflow instance list paging response
+ */
+public class WorkflowInstanceListPagingResponse extends Result {

Review Comment:
   done.



##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/dto/workflowInstance/WorkflowInstanceQueryRequest.java:
##########
@@ -0,0 +1,47 @@
+package org.apache.dolphinscheduler.api.dto.workflowInstance;
+
+import org.apache.dolphinscheduler.api.dto.PageQueryDto;
+import org.apache.dolphinscheduler.common.enums.TaskExecuteType;
+import org.apache.dolphinscheduler.common.enums.WorkflowExecutionStatus;
+
+import lombok.Data;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import io.swagger.v3.oas.annotations.media.Schema;
+
+/**
+ * workflow instance request
+ */
+@JsonIgnoreProperties(ignoreUnknown = true)
+@JsonInclude(JsonInclude.Include.NON_NULL)
+@Data
+public class WorkflowInstanceQueryRequest extends PageQueryDto {

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.

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

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

[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #12990: [Feature][API] New restful API for workflowInstance

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


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/WorkflowInstanceV2Controller.java:
##########
@@ -0,0 +1,129 @@
+package org.apache.dolphinscheduler.api.controller;
+
+import static org.apache.dolphinscheduler.api.enums.Status.*;

Review Comment:
   is this change by you or by spotless itself?



##########
dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapper.xml:
##########
@@ -274,6 +278,40 @@
         and id <![CDATA[ > ]]> #{id}
         order by id asc limit 1
     </select>
+    <select id="queryProcessInstanceListV2Paging"
+            resultType="org.apache.dolphinscheduler.dao.entity.ProcessInstance">
+        SELECT
+        instance.id
+        , instance.name, instance.process_definition_version, instance.process_definition_code, instance.state, instance.recovery, instance.start_time, instance.end_time, instance.run_times,host,
+        instance.command_type, instance.command_param, instance.task_depend_type, instance.max_try_times, instance.failure_strategy, instance.warning_type,
+        instance.warning_group_id, instance.schedule_time, instance.command_start_time, instance.global_params, instance.flag,
+        instance.update_time, instance.is_sub_process, instance.executor_id, instance.history_cmd,
+        instance.process_instance_priority, instance.worker_group,instance.environment_code, instance.timeout, instance.tenant_id, instance.var_pool,
+        instance.dry_run, instance.test_flag, instance.next_process_instance_id, instance.restart_time, instance.state_history

Review Comment:
   can we directly use 
   ```xml
           select
            <include refid="baseSql"/>
   ``` 
   or because we use other col, so we have to use custom col here?



##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/WorkflowInstanceV2Controller.java:
##########
@@ -0,0 +1,129 @@
+package org.apache.dolphinscheduler.api.controller;
+
+import static org.apache.dolphinscheduler.api.enums.Status.*;

Review Comment:
   we should avoid using `*` in our import



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

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

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

[GitHub] [dolphinscheduler] insist777 commented on a diff in pull request #12990: [Feature][API] New restful API for workflowInstance

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


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/dto/workflowInstance/WorkflowInstanceQueryRequest.java:
##########
@@ -0,0 +1,33 @@
+package org.apache.dolphinscheduler.api.dto.workflowInstance;
+
+import org.apache.dolphinscheduler.api.dto.PageQueryDto;
+import org.apache.dolphinscheduler.dao.entity.ProcessInstance;
+
+import lombok.Data;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import io.swagger.v3.oas.annotations.media.Schema;
+
+/**
+ * workflow instance request
+ */
+@JsonIgnoreProperties(ignoreUnknown = true)
+@JsonInclude(JsonInclude.Include.NON_NULL)
+@Data
+public class WorkflowInstanceQueryRequest extends PageQueryDto {
+
+    @Schema(name = "projectName", example = "PROJECT-NAME")
+    String projectName;
+
+    @Schema(name = "workflowName", example = "WORKFLOW-NAME")

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.

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

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

[GitHub] [dolphinscheduler] insist777 commented on pull request #12990: [Feature][API] New restful API for workflowInstance

Posted by GitBox <gi...@apache.org>.
insist777 commented on PR #12990:
URL: https://github.com/apache/dolphinscheduler/pull/12990#issuecomment-1327395996

   > may i ask if we need to modify the UI at the same time?
   
   The openapi does not need to modify the UI.
   


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

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

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

[GitHub] [dolphinscheduler] insist777 commented on a diff in pull request #12990: [Feature][API] New restful API for workflowInstance

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


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/WorkflowInstanceV2Controller.java:
##########
@@ -0,0 +1,129 @@
+package org.apache.dolphinscheduler.api.controller;
+
+import static org.apache.dolphinscheduler.api.enums.Status.*;

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.

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

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

[GitHub] [dolphinscheduler] zhongjiajie commented on pull request #12990: [Feature][API] New restful API for workflowInstance

Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on PR #12990:
URL: https://github.com/apache/dolphinscheduler/pull/12990#issuecomment-1328540060

   Looking good, 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.

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

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

[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #12990: [Feature][API] New restful API for workflowInstance

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

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12990)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=BUG) [![E](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/E-16px.png 'E')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=CODE_SMELL) [18 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12990&resolved=false&types=CODE_SMELL)
   
   [![15.5%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '15.5%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12990&metric=new_coverage&view=list) [15.5% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12990&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12990&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12990&metric=new_duplicated_lines_density&view=list)
   
   


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

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

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