-
Notifications
You must be signed in to change notification settings - Fork 4.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Improvement-15603][API] When removing or modifying a workflow the system can check if there any tasks depend on it. #15681
Conversation
}) | ||
@GetMapping(value = "/query-dependent-tasks") | ||
@ResponseStatus(HttpStatus.OK) | ||
public Result<Map<String, Object>> queryDownstreamDependentTaskList(@Parameter(hidden = true) @RequestAttribute(value = SESSION_USER) User loginUser, |
Check notice
Code scanning / CodeQL
Useless parameter Note
...eduler-api/src/main/java/org/apache/dolphinscheduler/api/service/WorkFlowLineageService.java
Fixed
Show fixed
Hide fixed
...src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java
Fixed
Show fixed
Hide fixed
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #15681 +/- ##
============================================
- Coverage 38.96% 38.96% -0.01%
- Complexity 4838 4840 +2
============================================
Files 1316 1316
Lines 45000 45010 +10
Branches 4817 4818 +1
============================================
+ Hits 17535 17538 +3
- Misses 25568 25576 +8
+ Partials 1897 1896 -1 ☔ View full report in Codecov by Sentry. |
} catch (Exception e) { | ||
log.error(QUERY_WORKFLOW_LINEAGE_ERROR.getMsg(), e); | ||
return error(QUERY_WORKFLOW_LINEAGE_ERROR.getCode(), QUERY_WORKFLOW_LINEAGE_ERROR.getMsg()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recommend using @ApiException
annotation for this part of the code.
/** | ||
* Check if there're any tasks that depend on the process | ||
*/ | ||
private void checkIfDependentTaskExists(ProcessDefinition processDefinition) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no called.
// const dependenciesData = reactive({ | ||
// showRef: false, | ||
// taskLinks: ref([]), | ||
// required: ref(false), | ||
// tip: ref(''), action: () => {} | ||
// }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please delete this part.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please delete introduced but unused variables
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…inscheduler into f-validate-dependences
Quality Gate failedFailed conditions |
It's odd that |
Maybe the command dolphinscheduler/.github/workflows/frontend.yml Lines 60 to 64 in 2395ba5
PTAL @songjianet |
Looks like some code format issue |
Purpose of the pull request
This PR will close #15603 .
Brief change log
Verify this pull request
This change added tests and can be verified as follows:
The test results are as follows: