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/09/28 04:54:53 UTC
[GitHub] [dolphinscheduler] fuchanghai opened a new pull request, #12177: [fix] fix problem about transaction (method of batchDeleteProcessDefinitionByCodes and batchDeleteProcessInstanceByIds)
fuchanghai opened a new pull request, #12177:
URL: https://github.com/apache/dolphinscheduler/pull/12177
<!--Thanks very much for contributing to Apache DolphinScheduler. Please review https://dolphinscheduler.apache.org/en-us/community/development/pull-request.html before opening a pull request.-->
## Purpose of the pull request
According to the previous code logic, it will cause partial success and partial failure
## Brief change log
## Verify this pull request
This pull request is code cleanup without any test coverage.
this close #11825
relation PR #11889
--
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] fuchanghai closed pull request #12177: [fix] fix problem about transaction (method of batchDeleteProcessDefinitionByCodes and batchDeleteProcessInstanceByIds)
Posted by GitBox <gi...@apache.org>.
fuchanghai closed pull request #12177: [fix] fix problem about transaction (method of batchDeleteProcessDefinitionByCodes and batchDeleteProcessInstanceByIds)
URL: https://github.com/apache/dolphinscheduler/pull/12177
--
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 #12177: [fix] fix problem about transaction (method of batchDeleteProcessDefinitionByCodes and batchDeleteProcessInstanceByIds)
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #12177:
URL: https://github.com/apache/dolphinscheduler/pull/12177#issuecomment-1260434294
# [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/12177?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 [#12177](https://codecov.io/gh/apache/dolphinscheduler/pull/12177?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (40a842a) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/d526dd89f66ddf43689ae544208b44a08ba04a40?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d526dd8) will **decrease** coverage by `0.00%`.
> The diff coverage is `50.00%`.
```diff
@@ Coverage Diff @@
## dev #12177 +/- ##
============================================
- Coverage 39.40% 39.39% -0.01%
+ Complexity 4128 4127 -1
============================================
Files 1010 1010
Lines 37817 37816 -1
Branches 4345 4345
============================================
- Hits 14903 14899 -4
- Misses 21188 21192 +4
+ Partials 1726 1725 -1
```
| [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/12177?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...er/api/controller/ProcessDefinitionController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12177/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvUHJvY2Vzc0RlZmluaXRpb25Db250cm9sbGVyLmphdmE=) | `51.04% <0.00%> (ø)` | |
| [...r/api/service/impl/ProcessInstanceServiceImpl.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12177/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvaW1wbC9Qcm9jZXNzSW5zdGFuY2VTZXJ2aWNlSW1wbC5qYXZh) | `57.84% <ø> (+0.13%)` | :arrow_up: |
| [...uler/api/controller/ProcessInstanceController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12177/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvUHJvY2Vzc0luc3RhbmNlQ29udHJvbGxlci5qYXZh) | `68.08% <100.00%> (-2.13%)` | :arrow_down: |
| [...olphinscheduler/api/controller/BaseController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12177/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvQmFzZUNvbnRyb2xsZXIuamF2YQ==) | `49.29% <0.00%> (-4.23%)` | :arrow_down: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #12177: [fix] fix problem about transaction (method of batchDeleteProcessDefinitionByCodes and batchDeleteProcessInstanceByIds)
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12177:
URL: https://github.com/apache/dolphinscheduler/pull/12177#issuecomment-1260442211
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12177)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12177&metric=new_coverage&view=list) [50.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12177&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12177&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12177&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] fuchanghai closed pull request #12177: [fix] fix problem about transaction (method of batchDeleteProcessDefinitionByCodes and batchDeleteProcessInstanceByIds)
Posted by GitBox <gi...@apache.org>.
fuchanghai closed pull request #12177: [fix] fix problem about transaction (method of batchDeleteProcessDefinitionByCodes and batchDeleteProcessInstanceByIds)
URL: https://github.com/apache/dolphinscheduler/pull/12177
--
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] fuchanghai commented on pull request #12177: [fix] fix problem about transaction (method of batchDeleteProcessDefinitionByCodes and batchDeleteProcessInstanceByIds)
Posted by GitBox <gi...@apache.org>.
fuchanghai commented on PR #12177:
URL: https://github.com/apache/dolphinscheduler/pull/12177#issuecomment-1272687937
@caishunfeng i will close 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] SbloodyS commented on a diff in pull request #12177: [fix] fix problem about transaction (method of batchDeleteProcessDefinitionByCodes and batchDeleteProcessInstanceByIds)
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #12177:
URL: https://github.com/apache/dolphinscheduler/pull/12177#discussion_r983182530
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ProcessDefinitionController.java:
##########
@@ -681,6 +682,7 @@ public Result deleteProcessDefinitionByCode(@ApiIgnore @RequestAttribute(value =
@ResponseStatus(HttpStatus.OK)
@ApiException(BATCH_DELETE_PROCESS_DEFINE_BY_CODES_ERROR)
@AccessLogAnnotation(ignoreRequestArgs = "loginUser")
+ @Transactional
Review Comment:
> BTW, I don't think this batch delete action should be transactional, because different workflows are independent of each other, not an atomic relationship. WDYT? @zhongjiajie @ruanwenjun @SbloodyS
+1
--
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 #12177: [fix] fix problem about transaction (method of batchDeleteProcessDefinitionByCodes and batchDeleteProcessInstanceByIds)
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12177:
URL: https://github.com/apache/dolphinscheduler/pull/12177#issuecomment-1260439115
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12177)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12177&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12177&metric=new_coverage&view=list) [50.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12177&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12177&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12177&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] caishunfeng commented on a diff in pull request #12177: [fix] fix problem about transaction (method of batchDeleteProcessDefinitionByCodes and batchDeleteProcessInstanceByIds)
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on code in PR #12177:
URL: https://github.com/apache/dolphinscheduler/pull/12177#discussion_r983177326
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ProcessDefinitionController.java:
##########
@@ -681,6 +682,7 @@ public Result deleteProcessDefinitionByCode(@ApiIgnore @RequestAttribute(value =
@ResponseStatus(HttpStatus.OK)
@ApiException(BATCH_DELETE_PROCESS_DEFINE_BY_CODES_ERROR)
@AccessLogAnnotation(ignoreRequestArgs = "loginUser")
+ @Transactional
Review Comment:
I think we should not add this annotation in controller layer.
It would better to create a batch delete method in service if we want to do that.
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ProcessDefinitionController.java:
##########
@@ -681,6 +682,7 @@ public Result deleteProcessDefinitionByCode(@ApiIgnore @RequestAttribute(value =
@ResponseStatus(HttpStatus.OK)
@ApiException(BATCH_DELETE_PROCESS_DEFINE_BY_CODES_ERROR)
@AccessLogAnnotation(ignoreRequestArgs = "loginUser")
+ @Transactional
Review Comment:
BTW, I don't think this batch delete action should be transactional, because different workflows are independent of each other, not an atomic relationship.
WDYT? @zhongjiajie @ruanwenjun @SbloodyS
--
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