You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2022/07/21 08:12:39 UTC
[GitHub] [dolphinscheduler] JinyLeeChina opened a new pull request, #11088: [Fix-11007] [Master] fix forced_success bug
JinyLeeChina opened a new pull request, #11088:
URL: https://github.com/apache/dolphinscheduler/pull/11088
closed by #11007
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] JinyLeeChina commented on a diff in pull request #11088: [Fix-11007] [Master] fix forced_success bug
Posted by GitBox <gi...@apache.org>.
JinyLeeChina commented on code in PR #11088:
URL: https://github.com/apache/dolphinscheduler/pull/11088#discussion_r926398530
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskInstanceServiceImpl.java:
##########
@@ -194,6 +198,24 @@ public Map<String, Object> forceTaskSuccess(User loginUser, long projectCode, In
return result;
}
+ ProcessInstance processInstance = processService.findProcessInstanceDetailById(task.getProcessInstanceId());
+ if (processInstance != null && (processInstance.getState().typeIsFailure() || processInstance.getState().typeIsCancel())) {
+ List<TaskInstance> validTaskList = processService.findValidTaskListByProcessId(processInstance.getId());
+ List<Long> instanceTaskCodeList = validTaskList.stream().map(TaskInstance::getTaskCode).collect(Collectors.toList());
+ List<ProcessTaskRelation> taskRelations = processService.findRelationByCode(processInstance.getProcessDefinitionCode(),
+ processInstance.getProcessDefinitionVersion());
+ List<TaskDefinitionLog> taskDefinitionLogs = processService.genTaskDefineList(taskRelations);
+ List<Long> definiteTaskCodeList = taskDefinitionLogs.stream().filter(definitionLog -> definitionLog.getFlag() == Flag.YES)
+ .map(TaskDefinitionLog::getCode).collect(Collectors.toList());
+ if (CollectionUtils.equalLists(instanceTaskCodeList, definiteTaskCodeList)) {
+ List<Integer> failTaskList = validTaskList.stream().filter(instance -> instance.getState().typeIsFailure() || instance.getState().typeIsCancel())
+ .map(TaskInstance::getId).collect(Collectors.toList());
+ if (failTaskList.size() == 1 && failTaskList.contains(taskInstanceId)) {
Review Comment:
Because the workflow should succeed only when the last failed task is forced to succeed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] JinyLeeChina commented on a diff in pull request #11088: [Fix-11007] [Master] fix forced_success bug
Posted by GitBox <gi...@apache.org>.
JinyLeeChina commented on code in PR #11088:
URL: https://github.com/apache/dolphinscheduler/pull/11088#discussion_r929470978
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskInstanceServiceImpl.java:
##########
@@ -193,7 +199,26 @@ public Map<String, Object> forceTaskSuccess(User loginUser, long projectCode, In
putMsg(result, Status.TASK_INSTANCE_STATE_OPERATION_ERROR, taskInstanceId, task.getState().toString());
return result;
}
-
+ // Workflow enforcement succeeded
+ ProcessInstance processInstance = processService.findProcessInstanceDetailById(task.getProcessInstanceId());
Review Comment:
The update workflow and update task are in the same transaction, and this code is difficult to be universal. So this was unnecessary
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] ruanwenjun commented on a diff in pull request #11088: [Fix-11007] [Master] fix forced_success bug
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #11088:
URL: https://github.com/apache/dolphinscheduler/pull/11088#discussion_r926630567
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskInstanceServiceImpl.java:
##########
@@ -194,6 +198,24 @@ public Map<String, Object> forceTaskSuccess(User loginUser, long projectCode, In
return result;
}
+ ProcessInstance processInstance = processService.findProcessInstanceDetailById(task.getProcessInstanceId());
+ if (processInstance != null && (processInstance.getState().typeIsFailure() || processInstance.getState().typeIsCancel())) {
+ List<TaskInstance> validTaskList = processService.findValidTaskListByProcessId(processInstance.getId());
+ List<Long> instanceTaskCodeList = validTaskList.stream().map(TaskInstance::getTaskCode).collect(Collectors.toList());
+ List<ProcessTaskRelation> taskRelations = processService.findRelationByCode(processInstance.getProcessDefinitionCode(),
+ processInstance.getProcessDefinitionVersion());
+ List<TaskDefinitionLog> taskDefinitionLogs = processService.genTaskDefineList(taskRelations);
+ List<Long> definiteTaskCodeList = taskDefinitionLogs.stream().filter(definitionLog -> definitionLog.getFlag() == Flag.YES)
+ .map(TaskDefinitionLog::getCode).collect(Collectors.toList());
+ if (CollectionUtils.equalLists(instanceTaskCodeList, definiteTaskCodeList)) {
+ List<Integer> failTaskList = validTaskList.stream().filter(instance -> instance.getState().typeIsFailure() || instance.getState().typeIsCancel())
+ .map(TaskInstance::getId).collect(Collectors.toList());
+ if (failTaskList.size() == 1 && failTaskList.contains(taskInstanceId)) {
Review Comment:
It's better to update the taskInstance and then judge if the workflowInstance is success, if the workflow success, we can update the workflow status to success.
The advantage of creating a method to judge a workflowInstance is success can be used in other place.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng commented on a diff in pull request #11088: [Fix-11007] [Master] fix forced_success bug
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on code in PR #11088:
URL: https://github.com/apache/dolphinscheduler/pull/11088#discussion_r926420148
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskInstanceServiceImpl.java:
##########
@@ -194,6 +198,24 @@ public Map<String, Object> forceTaskSuccess(User loginUser, long projectCode, In
return result;
}
+ ProcessInstance processInstance = processService.findProcessInstanceDetailById(task.getProcessInstanceId());
+ if (processInstance != null && (processInstance.getState().typeIsFailure() || processInstance.getState().typeIsCancel())) {
+ List<TaskInstance> validTaskList = processService.findValidTaskListByProcessId(processInstance.getId());
+ List<Long> instanceTaskCodeList = validTaskList.stream().map(TaskInstance::getTaskCode).collect(Collectors.toList());
+ List<ProcessTaskRelation> taskRelations = processService.findRelationByCode(processInstance.getProcessDefinitionCode(),
+ processInstance.getProcessDefinitionVersion());
+ List<TaskDefinitionLog> taskDefinitionLogs = processService.genTaskDefineList(taskRelations);
+ List<Long> definiteTaskCodeList = taskDefinitionLogs.stream().filter(definitionLog -> definitionLog.getFlag() == Flag.YES)
+ .map(TaskDefinitionLog::getCode).collect(Collectors.toList());
+ if (CollectionUtils.equalLists(instanceTaskCodeList, definiteTaskCodeList)) {
+ List<Integer> failTaskList = validTaskList.stream().filter(instance -> instance.getState().typeIsFailure() || instance.getState().typeIsCancel())
+ .map(TaskInstance::getId).collect(Collectors.toList());
+ if (failTaskList.size() == 1 && failTaskList.contains(taskInstanceId)) {
Review Comment:
I think it's better to handle processInstance after updating taskInstance, and then we don't need to use this judgment, WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng commented on a diff in pull request #11088: [Fix-11007] [Master] fix forced_success bug
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on code in PR #11088:
URL: https://github.com/apache/dolphinscheduler/pull/11088#discussion_r926396192
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskInstanceServiceImpl.java:
##########
@@ -194,6 +198,24 @@ public Map<String, Object> forceTaskSuccess(User loginUser, long projectCode, In
return result;
}
+ ProcessInstance processInstance = processService.findProcessInstanceDetailById(task.getProcessInstanceId());
+ if (processInstance != null && (processInstance.getState().typeIsFailure() || processInstance.getState().typeIsCancel())) {
+ List<TaskInstance> validTaskList = processService.findValidTaskListByProcessId(processInstance.getId());
+ List<Long> instanceTaskCodeList = validTaskList.stream().map(TaskInstance::getTaskCode).collect(Collectors.toList());
+ List<ProcessTaskRelation> taskRelations = processService.findRelationByCode(processInstance.getProcessDefinitionCode(),
+ processInstance.getProcessDefinitionVersion());
+ List<TaskDefinitionLog> taskDefinitionLogs = processService.genTaskDefineList(taskRelations);
+ List<Long> definiteTaskCodeList = taskDefinitionLogs.stream().filter(definitionLog -> definitionLog.getFlag() == Flag.YES)
+ .map(TaskDefinitionLog::getCode).collect(Collectors.toList());
+ if (CollectionUtils.equalLists(instanceTaskCodeList, definiteTaskCodeList)) {
+ List<Integer> failTaskList = validTaskList.stream().filter(instance -> instance.getState().typeIsFailure() || instance.getState().typeIsCancel())
+ .map(TaskInstance::getId).collect(Collectors.toList());
+ if (failTaskList.size() == 1 && failTaskList.contains(taskInstanceId)) {
Review Comment:
It looks strange, why is judgment = 1?
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskInstanceServiceImpl.java:
##########
@@ -194,6 +198,24 @@ public Map<String, Object> forceTaskSuccess(User loginUser, long projectCode, In
return result;
}
+ ProcessInstance processInstance = processService.findProcessInstanceDetailById(task.getProcessInstanceId());
Review Comment:
Please add some comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #11088: [Fix-11007] [Master] fix forced_success bug
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11088:
URL: https://github.com/apache/dolphinscheduler/pull/11088#issuecomment-1198943003
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=11088)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11088&metric=new_coverage&view=list) [2.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11088&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11088&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11088&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhuxt2015 commented on a diff in pull request #11088: [Fix-11007] [Master] fix forced_success bug
Posted by GitBox <gi...@apache.org>.
zhuxt2015 commented on code in PR #11088:
URL: https://github.com/apache/dolphinscheduler/pull/11088#discussion_r927264390
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskInstanceServiceImpl.java:
##########
@@ -193,7 +199,26 @@ public Map<String, Object> forceTaskSuccess(User loginUser, long projectCode, In
putMsg(result, Status.TASK_INSTANCE_STATE_OPERATION_ERROR, taskInstanceId, task.getState().toString());
return result;
}
-
+ // Workflow enforcement succeeded
+ ProcessInstance processInstance = processService.findProcessInstanceDetailById(task.getProcessInstanceId());
Review Comment:
Suggest extract a method
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #11088: [Fix-11007] [Master] fix forced_success bug
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11088:
URL: https://github.com/apache/dolphinscheduler/pull/11088#issuecomment-1198941455
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=11088)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11088&metric=new_coverage&view=list) [2.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11088&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11088&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11088&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on pull request #11088: [Fix-11007] [Master] fix forced_success bug
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on PR #11088:
URL: https://github.com/apache/dolphinscheduler/pull/11088#issuecomment-1197637681
restart the failed CI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng commented on pull request #11088: [Fix-11007] [Master] fix forced_success bug
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on PR #11088:
URL: https://github.com/apache/dolphinscheduler/pull/11088#issuecomment-1200226034
@zhongjiajie this pr need to cherry-pick to 3.0.0-prepare.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] JinyLeeChina commented on a diff in pull request #11088: [Fix-11007] [Master] fix forced_success bug
Posted by GitBox <gi...@apache.org>.
JinyLeeChina commented on code in PR #11088:
URL: https://github.com/apache/dolphinscheduler/pull/11088#discussion_r926585379
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskInstanceServiceImpl.java:
##########
@@ -194,6 +198,24 @@ public Map<String, Object> forceTaskSuccess(User loginUser, long projectCode, In
return result;
}
+ ProcessInstance processInstance = processService.findProcessInstanceDetailById(task.getProcessInstanceId());
+ if (processInstance != null && (processInstance.getState().typeIsFailure() || processInstance.getState().typeIsCancel())) {
+ List<TaskInstance> validTaskList = processService.findValidTaskListByProcessId(processInstance.getId());
+ List<Long> instanceTaskCodeList = validTaskList.stream().map(TaskInstance::getTaskCode).collect(Collectors.toList());
+ List<ProcessTaskRelation> taskRelations = processService.findRelationByCode(processInstance.getProcessDefinitionCode(),
+ processInstance.getProcessDefinitionVersion());
+ List<TaskDefinitionLog> taskDefinitionLogs = processService.genTaskDefineList(taskRelations);
+ List<Long> definiteTaskCodeList = taskDefinitionLogs.stream().filter(definitionLog -> definitionLog.getFlag() == Flag.YES)
+ .map(TaskDefinitionLog::getCode).collect(Collectors.toList());
+ if (CollectionUtils.equalLists(instanceTaskCodeList, definiteTaskCodeList)) {
+ List<Integer> failTaskList = validTaskList.stream().filter(instance -> instance.getState().typeIsFailure() || instance.getState().typeIsCancel())
+ .map(TaskInstance::getId).collect(Collectors.toList());
+ if (failTaskList.size() == 1 && failTaskList.contains(taskInstanceId)) {
Review Comment:
I don't think it's necessary. The update workflow and update task are in the same transaction
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #11088: [Fix-11007] [Master] fix forced_success bug
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11088:
URL: https://github.com/apache/dolphinscheduler/pull/11088#issuecomment-1191385663
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=11088)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11088&metric=new_coverage&view=list) [6.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11088&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11088&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11088&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] JinyLeeChina commented on a diff in pull request #11088: [Fix-11007] [Master] fix forced_success bug
Posted by GitBox <gi...@apache.org>.
JinyLeeChina commented on code in PR #11088:
URL: https://github.com/apache/dolphinscheduler/pull/11088#discussion_r929469111
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskInstanceServiceImpl.java:
##########
@@ -194,6 +198,24 @@ public Map<String, Object> forceTaskSuccess(User loginUser, long projectCode, In
return result;
}
+ ProcessInstance processInstance = processService.findProcessInstanceDetailById(task.getProcessInstanceId());
+ if (processInstance != null && (processInstance.getState().typeIsFailure() || processInstance.getState().typeIsCancel())) {
+ List<TaskInstance> validTaskList = processService.findValidTaskListByProcessId(processInstance.getId());
+ List<Long> instanceTaskCodeList = validTaskList.stream().map(TaskInstance::getTaskCode).collect(Collectors.toList());
+ List<ProcessTaskRelation> taskRelations = processService.findRelationByCode(processInstance.getProcessDefinitionCode(),
+ processInstance.getProcessDefinitionVersion());
+ List<TaskDefinitionLog> taskDefinitionLogs = processService.genTaskDefineList(taskRelations);
+ List<Long> definiteTaskCodeList = taskDefinitionLogs.stream().filter(definitionLog -> definitionLog.getFlag() == Flag.YES)
+ .map(TaskDefinitionLog::getCode).collect(Collectors.toList());
+ if (CollectionUtils.equalLists(instanceTaskCodeList, definiteTaskCodeList)) {
+ List<Integer> failTaskList = validTaskList.stream().filter(instance -> instance.getState().typeIsFailure() || instance.getState().typeIsCancel())
+ .map(TaskInstance::getId).collect(Collectors.toList());
+ if (failTaskList.size() == 1 && failTaskList.contains(taskInstanceId)) {
Review Comment:
I think this code is difficult to be universal
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on pull request #11088: [Fix-11007] [Master] fix forced_success bug
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on PR #11088:
URL: https://github.com/apache/dolphinscheduler/pull/11088#issuecomment-1197926537
@ruanwenjun do you have time to review this pr?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhuxt2015 commented on a diff in pull request #11088: [Fix-11007] [Master] fix forced_success bug
Posted by GitBox <gi...@apache.org>.
zhuxt2015 commented on code in PR #11088:
URL: https://github.com/apache/dolphinscheduler/pull/11088#discussion_r929614596
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskInstanceServiceImpl.java:
##########
@@ -193,7 +199,26 @@ public Map<String, Object> forceTaskSuccess(User loginUser, long projectCode, In
putMsg(result, Status.TASK_INSTANCE_STATE_OPERATION_ERROR, taskInstanceId, task.getState().toString());
return result;
}
-
+ // Workflow enforcement succeeded
+ ProcessInstance processInstance = processService.findProcessInstanceDetailById(task.getProcessInstanceId());
Review Comment:
Nobody wants to read long functions.Write the function shorter, the shorter the better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #11088: [Fix-11007] [Master] fix forced_success bug
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11088:
URL: https://github.com/apache/dolphinscheduler/pull/11088#issuecomment-1191382951
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=11088)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11088&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11088&metric=new_coverage&view=list) [6.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11088&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11088&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11088&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] ruanwenjun commented on a diff in pull request #11088: [Fix-11007] [Master] fix forced_success bug
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #11088:
URL: https://github.com/apache/dolphinscheduler/pull/11088#discussion_r926630567
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskInstanceServiceImpl.java:
##########
@@ -194,6 +198,24 @@ public Map<String, Object> forceTaskSuccess(User loginUser, long projectCode, In
return result;
}
+ ProcessInstance processInstance = processService.findProcessInstanceDetailById(task.getProcessInstanceId());
+ if (processInstance != null && (processInstance.getState().typeIsFailure() || processInstance.getState().typeIsCancel())) {
+ List<TaskInstance> validTaskList = processService.findValidTaskListByProcessId(processInstance.getId());
+ List<Long> instanceTaskCodeList = validTaskList.stream().map(TaskInstance::getTaskCode).collect(Collectors.toList());
+ List<ProcessTaskRelation> taskRelations = processService.findRelationByCode(processInstance.getProcessDefinitionCode(),
+ processInstance.getProcessDefinitionVersion());
+ List<TaskDefinitionLog> taskDefinitionLogs = processService.genTaskDefineList(taskRelations);
+ List<Long> definiteTaskCodeList = taskDefinitionLogs.stream().filter(definitionLog -> definitionLog.getFlag() == Flag.YES)
+ .map(TaskDefinitionLog::getCode).collect(Collectors.toList());
+ if (CollectionUtils.equalLists(instanceTaskCodeList, definiteTaskCodeList)) {
+ List<Integer> failTaskList = validTaskList.stream().filter(instance -> instance.getState().typeIsFailure() || instance.getState().typeIsCancel())
+ .map(TaskInstance::getId).collect(Collectors.toList());
+ if (failTaskList.size() == 1 && failTaskList.contains(taskInstanceId)) {
Review Comment:
It's better to update the taskInstance and then judge if the workflowInstance is success, if the workflow success, we can update the workflow status to success.
The advantage of creating a method to judge a workflowInstance success it that this method can be used in other place.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] codecov-commenter commented on pull request #11088: [Fix-11007] [Master] fix forced_success bug
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #11088:
URL: https://github.com/apache/dolphinscheduler/pull/11088#issuecomment-1198938072
# [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/11088?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#11088](https://codecov.io/gh/apache/dolphinscheduler/pull/11088?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (99cc826) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/594f00512b6dcda4cc9b6a51b25b1d818d17a422?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (594f005) will **decrease** coverage by `0.14%`.
> The diff coverage is `4.00%`.
```diff
@@ Coverage Diff @@
## dev #11088 +/- ##
============================================
- Coverage 40.30% 40.15% -0.15%
- Complexity 4854 4947 +93
============================================
Files 943 988 +45
Lines 37055 37706 +651
Branches 4073 4138 +65
============================================
+ Hits 14934 15140 +206
- Misses 20613 21025 +412
- Partials 1508 1541 +33
```
| [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/11088?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...nscheduler/service/process/ProcessServiceImpl.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11088/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3NlcnZpY2UvcHJvY2Vzcy9Qcm9jZXNzU2VydmljZUltcGwuamF2YQ==) | `28.79% <0.00%> (-0.51%)` | :arrow_down: |
| [...uler/api/service/impl/TaskInstanceServiceImpl.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11088/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvaW1wbC9UYXNrSW5zdGFuY2VTZXJ2aWNlSW1wbC5qYXZh) | `88.88% <100.00%> (+0.17%)` | :arrow_up: |
| [...erver/master/processor/queue/TaskEventService.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11088/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9wcm9jZXNzb3IvcXVldWUvVGFza0V2ZW50U2VydmljZS5qYXZh) | `69.64% <0.00%> (-10.72%)` | :arrow_down: |
| [.../apache/dolphinscheduler/api/utils/CheckUtils.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11088/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3V0aWxzL0NoZWNrVXRpbHMuamF2YQ==) | `65.51% <0.00%> (-10.35%)` | :arrow_down: |
| [...inscheduler/server/log/LoggerRequestProcessor.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11088/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1sb2ctc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3NlcnZlci9sb2cvTG9nZ2VyUmVxdWVzdFByb2Nlc3Nvci5qYXZh) | `19.14% <0.00%> (-2.81%)` | :arrow_down: |
| [...e/dolphinscheduler/remote/NettyRemotingClient.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11088/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL05ldHR5UmVtb3RpbmdDbGllbnQuamF2YQ==) | `50.00% <0.00%> (-2.78%)` | :arrow_down: |
| [...r/server/master/service/MasterFailoverService.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11088/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9zZXJ2aWNlL01hc3RlckZhaWxvdmVyU2VydmljZS5qYXZh) | `54.47% <0.00%> (-2.31%)` | :arrow_down: |
| [...che/dolphinscheduler/api/python/PythonGateway.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11088/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3B5dGhvbi9QeXRob25HYXRld2F5LmphdmE=) | `19.81% <0.00%> (-1.84%)` | :arrow_down: |
| [.../server/master/metrics/ProcessInstanceMetrics.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11088/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9tZXRyaWNzL1Byb2Nlc3NJbnN0YW5jZU1ldHJpY3MuamF2YQ==) | `57.57% <0.00%> (-1.80%)` | :arrow_down: |
| [...dolphinscheduler/remote/future/ResponseFuture.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11088/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL2Z1dHVyZS9SZXNwb25zZUZ1dHVyZS5qYXZh) | `81.96% <0.00%> (-1.64%)` | :arrow_down: |
| ... and [84 more](https://codecov.io/gh/apache/dolphinscheduler/pull/11088/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng merged pull request #11088: [Fix-11007] [Master] fix forced_success bug
Posted by GitBox <gi...@apache.org>.
caishunfeng merged PR #11088:
URL: https://github.com/apache/dolphinscheduler/pull/11088
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org