You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2020/07/10 14:33:05 UTC
[GitHub] [incubator-dolphinscheduler] RedemptionC opened a new pull request #3184: add feature: query top n process instance order by running duration
RedemptionC opened a new pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184
add feature: query top n process instance order by running duration
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-dolphinscheduler] yangyichao-mango commented on a change in pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
yangyichao-mango commented on a change in pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#discussion_r453151806
##########
File path: dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapperTest.java
##########
@@ -51,6 +51,23 @@
@Autowired
ProjectMapper projectMapper;
+ /**
+ * insert process instance with specified start time and end time,set state to SUCCESS
+ * @param startTime
+ * @param endTime
+ * @return
+ */
+ private ProcessInstance insertOne(Date startTime,Date endTime){
Review comment:
```suggestion
private ProcessInstance insertOne(Date startTime, Date endTime) {
```
Please configure the checkstyle.xml plugin[1] to your IDE to format code.
[1] https://github.com/apache/incubator-dolphinscheduler/tree/dev/style
----------------------------------------------------------------
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 #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#issuecomment-656748626
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=3184&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=3184&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&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=3184&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&metric=coverage&view=list) No Coverage information
[<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=3184&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&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 #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#issuecomment-657093818
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=3184&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=3184&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&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=3184&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&metric=coverage&view=list) No Coverage information
[<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=3184&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&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] commented on pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#issuecomment-656814317
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=3184&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=3184&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&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=3184&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&metric=coverage&view=list) No Coverage information
[<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=3184&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&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] RedemptionC commented on a change in pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
RedemptionC commented on a change in pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#discussion_r453313064
##########
File path: dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/ProcessService.java
##########
@@ -1853,4 +1937,16 @@ public String formatTaskAppId(TaskInstance taskInstance){
taskInstance.getId());
}
+ /**
+ * return top n SUCCESS process instance order by running time which started between startTime and endTime
+ *
+ * @param size
+ * @param startTime
+ * @param endTime
+ * @return
+ */
+ public List<ProcessInstance> topNLongestRunningProcessInstance(int size, Date startTime, Date endTime) {
Review comment:
there are no NEGATIVE_SIZE_NUMBER_ERROR or START_TIME_BIGGER_THAN_END_TIME_ERROR in Status , should I modify Status.java?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-dolphinscheduler] yangyichao-mango commented on a change in pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
yangyichao-mango commented on a change in pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#discussion_r453275501
##########
File path: dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/ProcessService.java
##########
@@ -1853,4 +1937,16 @@ public String formatTaskAppId(TaskInstance taskInstance){
taskInstance.getId());
}
+ /**
+ * return top n SUCCESS process instance order by running time which started between startTime and endTime
+ *
+ * @param size
+ * @param startTime
+ * @param endTime
+ * @return
+ */
+ public List<ProcessInstance> topNLongestRunningProcessInstance(int size, Date startTime, Date endTime) {
Review comment:
```suggestion
public List<ProcessInstance> listTopNLongestRunningProcessInstances(int size, Date startTime, Date endTime) {
```
Is it better to add a negative number check of `size`?
----------------------------------------------------------------
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 #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#issuecomment-656814317
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=3184&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=3184&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&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=3184&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&metric=coverage&view=list) No Coverage information
[<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=3184&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&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] commented on pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#issuecomment-657095897
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=3184&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=3184&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&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=3184&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&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=3184&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&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] yangyichao-mango commented on a change in pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
yangyichao-mango commented on a change in pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#discussion_r453151529
##########
File path: dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapper.xml
##########
@@ -37,6 +37,16 @@
order by id asc
</select>
+ <select id="queryTopNProcessInstance" resultType="org.apache.dolphinscheduler.dao.entity.ProcessInstance">
+ select *
+ from t_ds_process_instance
+ where state = 7
+ and start_time between
+ #{startTime} and #{endTime}
+ order by end_time-start_time desc
+ limit #{size}
+ </select>
Review comment:
```suggestion
<select id="queryTopNProcessInstance" resultType="org.apache.dolphinscheduler.dao.entity.ProcessInstance">
select *
from `t_ds_process_instance`
where `state` = #{state}
and `start_time` between
#{startTime} and #{endTime}
order by `end_time` - `start_time` desc
limit #{size}
</select>
```
Hi,
Good job, is it better to pass the `state` as a parameter to avoid the value change of state.
Thx a lot.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-dolphinscheduler] yangyichao-mango commented on a change in pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
yangyichao-mango commented on a change in pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#discussion_r453200205
##########
File path: dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapper.xml
##########
@@ -37,6 +37,16 @@
order by id asc
</select>
+ <select id="queryTopNProcessInstance" resultType="org.apache.dolphinscheduler.dao.entity.ProcessInstance">
+ select *
+ from t_ds_process_instance
+ where state = 7
+ and start_time between
+ #{startTime} and #{endTime}
+ order by end_time-start_time desc
+ limit #{size}
+ </select>
Review comment:
> I still have a question about the first previous question: since Java doesn't have default params,when I want to set SUCCESS as the default status,I might overload the method like this:
> ` List<ProcessInstance> queryTopNProcessInstance(@Param("size") int size, @Param("startTime") Date startTime, @Param("endTime") Date endTime);`
>
> ` List<ProcessInstance> queryTopNProcessInstance(@Param("size") int size, @Param("startTime") Date startTime, @Param("endTime") Date endTime, @Param("status")ExecutionStatus status);`
> but since there is no method body here,how do I set SUCCESS as the default status?
> I am not so familiar with Java,I hope you can give me some advice
Hi,
Thx a lot for your answer and your contribution.
Can you tell me where you used `queryTopNProcessInstance` method except test part? This method was not used elsewhere according to your Pull Request.
According to your issue #3191 , if I understand it correctly, the purpose of this issue is to show the top-N tasks in the web interface. But at present, this PR does not seem to implement the issue completely. Are you still in the process of completing it?
----------------------
你可以告诉我在除了测试部分的其他哪些地方用到了 `queryTopNProcessInstance` 吗？根据你的 pr 来看，这个方法似乎没有被其他地方引用到。
根据你的 issue 来看，如果我理解正确的话，目的是想在web界面上展示运行时长topn的任务。但是目前这个 pr 似乎没有完全完成那个 issue，你是还在完成的过程中吗？
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-dolphinscheduler] yangyichao-mango commented on a change in pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
yangyichao-mango commented on a change in pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#discussion_r453275374
##########
File path: dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/ProcessService.java
##########
@@ -1809,42 +1889,46 @@ public String getTaskWorkerGroup(TaskInstance taskInstance) {
/**
* get user by user id
+ *
* @param userId user id
* @return User
*/
- public User getUserById(int userId){
+ public User getUserById(int userId) {
return userMapper.selectById(userId);
}
/**
* get resource by resoruce id
+ *
* @param resoruceId resource id
* @return Resource
*/
- public Resource getResourceById(int resoruceId){
+ public Resource getResourceById(int resoruceId) {
return resourceMapper.selectById(resoruceId);
}
/**
* list resources by ids
+ *
* @param resIds resIds
* @return resource list
*/
- public List<Resource> listResourceByIds(Integer[] resIds){
+ public List<Resource> listResourceByIds(Integer[] resIds) {
return resourceMapper.listResourceByIds(resIds);
}
/**
* format task app id in task instance
+ *
* @param taskInstance
* @return
*/
- public String formatTaskAppId(TaskInstance taskInstance){
+ public String formatTaskAppId(TaskInstance taskInstance) {
ProcessDefinition definition = this.findProcessDefineById(taskInstance.getProcessDefinitionId());
ProcessInstance processInstanceById = this.findProcessInstanceById(taskInstance.getProcessInstanceId());
- if(definition == null || processInstanceById == null){
+ if (definition == null || processInstanceById == null) {
Review comment:
Hi,
Please don't change so many code format problems unrelated to your Pull Request. If you want to format the code, you can commit a new Pull Request.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-dolphinscheduler] yangyichao-mango commented on a change in pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
yangyichao-mango commented on a change in pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#discussion_r453200205
##########
File path: dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapper.xml
##########
@@ -37,6 +37,16 @@
order by id asc
</select>
+ <select id="queryTopNProcessInstance" resultType="org.apache.dolphinscheduler.dao.entity.ProcessInstance">
+ select *
+ from t_ds_process_instance
+ where state = 7
+ and start_time between
+ #{startTime} and #{endTime}
+ order by end_time-start_time desc
+ limit #{size}
+ </select>
Review comment:
> I still have a question about the first previous question: since Java doesn't have default params,when I want to set SUCCESS as the default status,I might overload the method like this:
> ` List<ProcessInstance> queryTopNProcessInstance(@Param("size") int size, @Param("startTime") Date startTime, @Param("endTime") Date endTime);`
>
> ` List<ProcessInstance> queryTopNProcessInstance(@Param("size") int size, @Param("startTime") Date startTime, @Param("endTime") Date endTime, @Param("status")ExecutionStatus status);`
> but since there is no method body here,how do I set SUCCESS as the default status?
> I am not so familiar with Java,I hope you can give me some advice
Hi,
Thx a lot for your answer and your contribution.
Can you tell me where you used `queryTopNProcessInstance` method except test part? This method was not used elsewhere according to your Pull Request.
According to your issue #3191 , if I understand it correctly, the purpose of this issue is to show the top-N tasks in the web interface. But at present, this PR does not seem to implement the issue completely. Are you still in the process of completing it?
If you have any questions and suggestions, please put forward.
----------------------
你可以告诉我在除了测试部分的其他哪些地方用到了 `queryTopNProcessInstance` 吗？根据你的 pr 来看，这个方法似乎没有被其他地方引用到。
根据你的 issue 来看，如果我理解正确的话，目的是想在web界面上展示运行时长topn的任务。但是目前这个 pr 似乎没有完全完成那个 issue，你是还在完成的过程中吗？
如果有其他问题或者疑问，敬请提出。
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-dolphinscheduler] yangyichao-mango commented on pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
yangyichao-mango commented on pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#issuecomment-656983046
> add feature: query top n process instance order by running duration
Hi,
Very good job and thx a lot for your contribution.
Please create an issue and link this pr to the issue to help us to track the feature.
Thx a lot.
----------------------------------------------------------------
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 #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#issuecomment-656748626
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=3184&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=3184&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&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=3184&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&metric=coverage&view=list) No Coverage information
[<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=3184&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&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] commented on pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#issuecomment-657093818
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=3184&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=3184&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&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=3184&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&metric=coverage&view=list) No Coverage information
[<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=3184&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&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] RedemptionC commented on a change in pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
RedemptionC commented on a change in pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#discussion_r453213755
##########
File path: dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapper.xml
##########
@@ -37,6 +37,16 @@
order by id asc
</select>
+ <select id="queryTopNProcessInstance" resultType="org.apache.dolphinscheduler.dao.entity.ProcessInstance">
+ select *
+ from t_ds_process_instance
+ where state = 7
+ and start_time between
+ #{startTime} and #{endTime}
+ order by end_time-start_time desc
+ limit #{size}
+ </select>
Review comment:
I probably figured out how to make it: make status a param,and always call it with status SUCCESS(until I don't want to)
----------------------------------------------------------------
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] RedemptionC commented on a change in pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
RedemptionC commented on a change in pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#discussion_r453281323
##########
File path: dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/ProcessService.java
##########
@@ -1853,4 +1937,16 @@ public String formatTaskAppId(TaskInstance taskInstance){
taskInstance.getId());
}
+ /**
+ * return top n SUCCESS process instance order by running time which started between startTime and endTime
+ *
+ * @param size
+ * @param startTime
+ * @param endTime
+ * @return
+ */
+ public List<ProcessInstance> topNLongestRunningProcessInstance(int size, Date startTime, Date endTime) {
Review comment:
can I do the check like this?
` if(size<=0||startTime==null||endTime==null||startTime.getTime()>endTime.getTime()){
return new ArrayList<ProcessInstance>();
} `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-dolphinscheduler] yangyichao-mango commented on a change in pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
yangyichao-mango commented on a change in pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#discussion_r453192334
##########
File path: dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapper.xml
##########
@@ -37,6 +37,16 @@
order by id asc
</select>
+ <select id="queryTopNProcessInstance" resultType="org.apache.dolphinscheduler.dao.entity.ProcessInstance">
+ select *
+ from t_ds_process_instance
+ where state = 7
+ and start_time between
+ #{startTime} and #{endTime}
+ order by end_time-start_time desc
+ limit #{size}
+ </select>
Review comment:
> I have 2 questions:
> 1.if we only want to query SUCCESS process instance,do we need a state parameter ? If we need it,how do we put the parameter into the mapper function?
> 2.why do we need **`** in this case since other cases don't use them?
Hi,
Thx a lot for your answer and your contribution.
For the first question, what I consider is to avoid changing the status code of `ExecutionStatus.SUCCESS`, for example, if you pass the code throw `ExecutionStatus.SUCCESS.getCode()`, it will avoid the error scene from changing the code of `ExecutionStatus.SUCCESS` from 7 to 6.
You can refer to [1] to look over the example of passing the parameter.
For the second question, using ` is actually a good programming habit, which can avoid conflicts between SQL field parsing and keywords in SQL Engine, and avoid some unforeseen mistakes.
If you have any questions and suggestions, please put forward.
[1] https://github.com/apache/incubator-dolphinscheduler/blob/dev/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapMapper.java
----------------------------------------------------------------
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] RedemptionC commented on a change in pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
RedemptionC commented on a change in pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#discussion_r457332403
##########
File path: dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/ProcessService.java
##########
@@ -1853,4 +1937,16 @@ public String formatTaskAppId(TaskInstance taskInstance){
taskInstance.getId());
}
+ /**
+ * return top n SUCCESS process instance order by running time which started between startTime and endTime
+ *
+ * @param size
+ * @param startTime
+ * @param endTime
+ * @return
+ */
+ public List<ProcessInstance> topNLongestRunningProcessInstance(int size, Date startTime, Date endTime) {
Review comment:
> > can I do the check like this?
> > ` if(size<=0||startTime==null||endTime==null||startTime.getTime()>endTime.getTime()){ return new ArrayList<ProcessInstance>(); }`
>
> Hi,
> You should write this method in `org.apache.dolphinscheduler.api.service.ProcessInstanceService` of `dolphinscheduler-api` module. And you can handle this in below template[1] like the other method in `ProcessInstanceService` if you like.
> It will be better to add the comment in every method.
> Thx a lot for your contribution.
>
> If you have any question or suggestion, please put forward.
>
> [1] template:
>
> ```
> public Map<String, Object> listTopNLongestRunningProcessInstances(int size, Date startTime, Date endTime) {
>
> Map<String, Object> result = new HashMap<>();
>
> if (0 > size) {
> putMsg(result, Status.NEGATIVE_SIZE_NUMBER_ERROR, size);
> return result;
> }
> if (Objects.isNull(startTime)) {
> putMsg(result, Status.DATA_IS_NULL, Constants.START_TIME);
> return result;
> }
> if (Objects.isNull(endTime)) {
> putMsg(result, Status.DATA_IS_NULL, Constants.END_TIME);
> return result;
> }
> if (startTime.getTime() > endTime.getTime()) {
> putMsg(result, Status.START_TIME_BIGGER_THAN_END_TIME_ERROR, startTime, endTime);
> return result;
> }
>
> List<ProcessInstance> processInstances = processInstanceMapper.queryTopNProcessInstance(size, startTime, endTime, ExecutionStatus.SUCCESS);
>
> result.put(Constants.DATA_LIST, processInstances);
> putMsg(result, Status.SUCCESS);
> return result;
> }
> ```
I wonder what should I test in corresponding test method?
in processInstanceServiceTest.java
----------------------------------------------------------------
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 #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#issuecomment-657095897
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=3184&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=3184&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&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=3184&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&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=3184&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&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] yangyichao-mango commented on a change in pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
yangyichao-mango commented on a change in pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#discussion_r453192334
##########
File path: dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapper.xml
##########
@@ -37,6 +37,16 @@
order by id asc
</select>
+ <select id="queryTopNProcessInstance" resultType="org.apache.dolphinscheduler.dao.entity.ProcessInstance">
+ select *
+ from t_ds_process_instance
+ where state = 7
+ and start_time between
+ #{startTime} and #{endTime}
+ order by end_time-start_time desc
+ limit #{size}
+ </select>
Review comment:
> I have 2 questions:
> 1.if we only want to query SUCCESS process instance,do we need a state parameter ? If we need it,how do we put the parameter into the mapper function?
> 2.why do we need **`** in this case since other cases don't use them?
Hi,
Thx a lot for your answer and your contribution.
For the first question, what I consider is to avoid changing the status code of `ExecutionStatus.SUCCESS`, for example, if you pass the code throw `ExecutionStatus.SUCCESS.getCode()`, it will avoid the error scene from changing the code of `ExecutionStatus.SUCCESS` from 7 to 6.
You can refer to [1] to look over the example of passing the parameter.
For the second question, using ``` is actually a good programming habit, which can avoid conflicts between SQL field parsing and keywords in SQL Engine.
[1] https://github.com/apache/incubator-dolphinscheduler/blob/dev/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapMapper.java
##########
File path: dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapper.xml
##########
@@ -37,6 +37,16 @@
order by id asc
</select>
+ <select id="queryTopNProcessInstance" resultType="org.apache.dolphinscheduler.dao.entity.ProcessInstance">
+ select *
+ from t_ds_process_instance
+ where state = 7
+ and start_time between
+ #{startTime} and #{endTime}
+ order by end_time-start_time desc
+ limit #{size}
+ </select>
Review comment:
> I have 2 questions:
> 1.if we only want to query SUCCESS process instance,do we need a state parameter ? If we need it,how do we put the parameter into the mapper function?
> 2.why do we need **`** in this case since other cases don't use them?
Hi,
Thx a lot for your answer and your contribution.
For the first question, what I consider is to avoid changing the status code of `ExecutionStatus.SUCCESS`, for example, if you pass the code throw `ExecutionStatus.SUCCESS.getCode()`, it will avoid the error scene from changing the code of `ExecutionStatus.SUCCESS` from 7 to 6.
You can refer to [1] to look over the example of passing the parameter.
For the second question, using ` is actually a good programming habit, which can avoid conflicts between SQL field parsing and keywords in SQL Engine.
[1] https://github.com/apache/incubator-dolphinscheduler/blob/dev/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapMapper.java
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-dolphinscheduler] yangyichao-mango commented on a change in pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
yangyichao-mango commented on a change in pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#discussion_r453192334
##########
File path: dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapper.xml
##########
@@ -37,6 +37,16 @@
order by id asc
</select>
+ <select id="queryTopNProcessInstance" resultType="org.apache.dolphinscheduler.dao.entity.ProcessInstance">
+ select *
+ from t_ds_process_instance
+ where state = 7
+ and start_time between
+ #{startTime} and #{endTime}
+ order by end_time-start_time desc
+ limit #{size}
+ </select>
Review comment:
> I have 2 questions:
> 1.if we only want to query SUCCESS process instance,do we need a state parameter ? If we need it,how do we put the parameter into the mapper function?
> 2.why do we need **`** in this case since other cases don't use them?
Hi,
Thx a lot for your answer and your contribution.
For the first question, what I consider is to avoid changing the status code of `ExecutionStatus.SUCCESS`, for example, if you pass the code throw `ExecutionStatus.SUCCESS.getCode()`, it will avoid the error scene from changing the code of `ExecutionStatus.SUCCESS` from 7 to 6.
You can refer to [1] to look over the example of passing the parameter.
For the second question, using ` is actually a good programming habit, which can avoid conflicts between SQL field parsing and keywords in SQL Engine, and avoid some unforeseen mistakes
[1] https://github.com/apache/incubator-dolphinscheduler/blob/dev/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapMapper.java
##########
File path: dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapper.xml
##########
@@ -37,6 +37,16 @@
order by id asc
</select>
+ <select id="queryTopNProcessInstance" resultType="org.apache.dolphinscheduler.dao.entity.ProcessInstance">
+ select *
+ from t_ds_process_instance
+ where state = 7
+ and start_time between
+ #{startTime} and #{endTime}
+ order by end_time-start_time desc
+ limit #{size}
+ </select>
Review comment:
> I have 2 questions:
> 1.if we only want to query SUCCESS process instance,do we need a state parameter ? If we need it,how do we put the parameter into the mapper function?
> 2.why do we need **`** in this case since other cases don't use them?
Hi,
Thx a lot for your answer and your contribution.
For the first question, what I consider is to avoid changing the status code of `ExecutionStatus.SUCCESS`, for example, if you pass the code throw `ExecutionStatus.SUCCESS.getCode()`, it will avoid the error scene from changing the code of `ExecutionStatus.SUCCESS` from 7 to 6.
You can refer to [1] to look over the example of passing the parameter.
For the second question, using ` is actually a good programming habit, which can avoid conflicts between SQL field parsing and keywords in SQL Engine, and avoid some unforeseen mistakes.
[1] https://github.com/apache/incubator-dolphinscheduler/blob/dev/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapMapper.java
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-dolphinscheduler] yangyichao-mango commented on a change in pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
yangyichao-mango commented on a change in pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#discussion_r453298636
##########
File path: dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/ProcessService.java
##########
@@ -1853,4 +1937,16 @@ public String formatTaskAppId(TaskInstance taskInstance){
taskInstance.getId());
}
+ /**
+ * return top n SUCCESS process instance order by running time which started between startTime and endTime
+ *
+ * @param size
+ * @param startTime
+ * @param endTime
+ * @return
+ */
+ public List<ProcessInstance> topNLongestRunningProcessInstance(int size, Date startTime, Date endTime) {
Review comment:
> can I do the check like this?
> ` if(size<=0||startTime==null||endTime==null||startTime.getTime()>endTime.getTime()){ return new ArrayList<ProcessInstance>(); }`
Hi,
You should write this method in `org.apache.dolphinscheduler.api.service.ProcessInstanceService` of `dolphinscheduler-api` module. And you can handle this in below template[1] like the other method in `ProcessInstanceService` if you like.
It will be better to add the comment in every method.
Thx a lot for your contribution.
If you have any question or suggestion, please put forward.
[1] template:
```
public Map<String, Object> listTopNLongestRunningProcessInstances(int size, Date startTime, Date endTime) {
Map<String, Object> result = new HashMap<>();
if (0 > size) {
putMsg(result, Status.NEGATIVE_SIZE_NUMBER_ERROR, size);
return result;
}
if (Objects.isNull(startTime)) {
putMsg(result, Status.DATA_IS_NULL, Constants.START_TIME);
return result;
}
if (Objects.isNull(endTime)) {
putMsg(result, Status.DATA_IS_NULL, Constants.END_TIME);
return result;
}
if (startTime.getTime() > endTime.getTime()) {
putMsg(result, Status.START_TIME_BIGGER_THAN_END_TIME_ERROR, startTime, endTime);
return result;
}
List<ProcessInstance> processInstances = processInstanceMapper.queryTopNProcessInstance(size, startTime, endTime, ExecutionStatus.SUCCESS);
result.put(Constants.DATA_LIST, processInstances);
putMsg(result, Status.SUCCESS);
return result;
}
```
----------------------------------------------------------------
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] RedemptionC commented on a change in pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
RedemptionC commented on a change in pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#discussion_r453281425
##########
File path: dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/ProcessService.java
##########
@@ -1809,42 +1889,46 @@ public String getTaskWorkerGroup(TaskInstance taskInstance) {
/**
* get user by user id
+ *
* @param userId user id
* @return User
*/
- public User getUserById(int userId){
+ public User getUserById(int userId) {
return userMapper.selectById(userId);
}
/**
* get resource by resoruce id
+ *
* @param resoruceId resource id
* @return Resource
*/
- public Resource getResourceById(int resoruceId){
+ public Resource getResourceById(int resoruceId) {
return resourceMapper.selectById(resoruceId);
}
/**
* list resources by ids
+ *
* @param resIds resIds
* @return resource list
*/
- public List<Resource> listResourceByIds(Integer[] resIds){
+ public List<Resource> listResourceByIds(Integer[] resIds) {
return resourceMapper.listResourceByIds(resIds);
}
/**
* format task app id in task instance
+ *
* @param taskInstance
* @return
*/
- public String formatTaskAppId(TaskInstance taskInstance){
+ public String formatTaskAppId(TaskInstance taskInstance) {
ProcessDefinition definition = this.findProcessDefineById(taskInstance.getProcessDefinitionId());
ProcessInstance processInstanceById = this.findProcessInstanceById(taskInstance.getProcessInstanceId());
- if(definition == null || processInstanceById == null){
+ if (definition == null || processInstanceById == null) {
Review comment:
ok I'll fix it in next commit
----------------------------------------------------------------
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 #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#issuecomment-656713831
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=3184&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=3184&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&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=3184&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&metric=coverage&view=list) No Coverage information
[<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=3184&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&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 #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#issuecomment-656712262
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=3184&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=3184&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&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=3184&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&metric=coverage&view=list) No Coverage information
[<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=3184&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&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] commented on pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#issuecomment-657020331
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=3184&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=3184&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&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=3184&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&metric=coverage&view=list) No Coverage information
[<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=3184&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&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] RedemptionC commented on a change in pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
RedemptionC commented on a change in pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#discussion_r453170361
##########
File path: dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapperTest.java
##########
@@ -51,6 +51,23 @@
@Autowired
ProjectMapper projectMapper;
+ /**
+ * insert process instance with specified start time and end time,set state to SUCCESS
+ * @param startTime
+ * @param endTime
+ * @return
+ */
+ private ProcessInstance insertOne(Date startTime,Date endTime){
Review comment:
ok , I will fix it
----------------------------------------------------------------
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 #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#issuecomment-657020331
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=3184&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=3184&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&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=3184&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&metric=coverage&view=list) No Coverage information
[<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=3184&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&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] commented on pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#issuecomment-656713831
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=3184&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=3184&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&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=3184&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&metric=coverage&view=list) No Coverage information
[<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=3184&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&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] commented on pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#issuecomment-657099993
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=3184&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=3184&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&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=3184&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&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=3184&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&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] RedemptionC commented on a change in pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
RedemptionC commented on a change in pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#discussion_r453194020
##########
File path: dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapper.xml
##########
@@ -37,6 +37,16 @@
order by id asc
</select>
+ <select id="queryTopNProcessInstance" resultType="org.apache.dolphinscheduler.dao.entity.ProcessInstance">
+ select *
+ from t_ds_process_instance
+ where state = 7
+ and start_time between
+ #{startTime} and #{endTime}
+ order by end_time-start_time desc
+ limit #{size}
+ </select>
Review comment:
I still have a question about the first previous question: since Java doesn't have default params,when I want to set SUCCESS as the default status,I might overload the method like this:
` List<ProcessInstance> queryTopNProcessInstance(@Param("size") int size,
@Param("startTime") Date startTime,
@Param("endTime") Date endTime);`
` List<ProcessInstance> queryTopNProcessInstance(@Param("size") int size,
@Param("startTime") Date startTime,
@Param("endTime") Date endTime,
@Param("status")ExecutionStatus status);`
but since there is no method body here,how do I set SUCCESS as the default status?
I am not so familiar with Java,I hope you can give me some advice
----------------------------------------------------------------
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 #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#issuecomment-656712262
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=3184&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=3184&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&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=3184&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&metric=coverage&view=list) No Coverage information
[<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=3184&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&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] RedemptionC closed pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
RedemptionC closed pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184
----------------------------------------------------------------
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 #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#issuecomment-656741152
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=3184&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=3184&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&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=3184&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&metric=coverage&view=list) No Coverage information
[<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=3184&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&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] RedemptionC commented on a change in pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
RedemptionC commented on a change in pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#discussion_r453170770
##########
File path: dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapper.xml
##########
@@ -37,6 +37,16 @@
order by id asc
</select>
+ <select id="queryTopNProcessInstance" resultType="org.apache.dolphinscheduler.dao.entity.ProcessInstance">
+ select *
+ from t_ds_process_instance
+ where state = 7
+ and start_time between
+ #{startTime} and #{endTime}
+ order by end_time-start_time desc
+ limit #{size}
+ </select>
Review comment:
I have 2 questions:
1.if we only want to query SUCCESS process instance,do we need a state parameter ? If we need it,how do we put the parameter into the mapper function?
2.why do we need **`** in this case since other cases don't use them?
----------------------------------------------------------------
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 #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#issuecomment-656741152
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=3184&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=3184&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&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=3184&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=SECURITY_HOTSPOT) to review)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&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=3184&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=3184&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&metric=coverage&view=list) No Coverage information
[<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=3184&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=3184&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] yangyichao-mango commented on a change in pull request #3184: add feature: query top n process instance order by running duration
Posted by GitBox <gi...@apache.org>.
yangyichao-mango commented on a change in pull request #3184:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/3184#discussion_r453151529
##########
File path: dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProcessInstanceMapper.xml
##########
@@ -37,6 +37,16 @@
order by id asc
</select>
+ <select id="queryTopNProcessInstance" resultType="org.apache.dolphinscheduler.dao.entity.ProcessInstance">
+ select *
+ from t_ds_process_instance
+ where state = 7
+ and start_time between
+ #{startTime} and #{endTime}
+ order by end_time-start_time desc
+ limit #{size}
+ </select>
Review comment:
```suggestion
<select id="queryTopNProcessInstance" resultType="org.apache.dolphinscheduler.dao.entity.ProcessInstance">
select *
from `t_ds_process_instance`
where `state` = #{state}
and `start_time` between
#{startTime} and #{endTime}
order by `end_time` - `start_time` desc
limit #{size}
</select>
```
Hi,
Good job and thx a lot for your contribution, is it better to pass the `state` as a parameter to avoid the value change of state.
----------------------------------------------------------------
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