You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2022/11/22 11:05:00 UTC
[GitHub] [dolphinscheduler] Zzih opened a new pull request, #12969: [Feature][Api] Refactor org.apache.dolphinscheduler.api.controller.DataAnalysisController
Zzih opened a new pull request, #12969:
URL: https://github.com/apache/dolphinscheduler/pull/12969
## Purpose of the pull request
Fix https://github.com/apache/dolphinscheduler/issues/11754
--
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] github-code-scanning[bot] commented on a diff in pull request #12969: [Feature][Api] Refactor org.apache.dolphinscheduler.api.controller.DataAnalysisController
Posted by GitBox <gi...@apache.org>.
github-code-scanning[bot] commented on code in PR #12969:
URL: https://github.com/apache/dolphinscheduler/pull/12969#discussion_r1029230433
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/DataAnalysisServiceImpl.java:
##########
@@ -153,13 +154,16 @@
* @param startDate start date
* @param endDate end date
*/
- private Map<String, Object> countStateByProject(User loginUser, long projectCode, String startDate, String endDate,
- TriFunction<Date, Date, Long[], List<ExecuteStatusCount>> instanceStateCounter) {
- Map<String, Object> result = new HashMap<>();
+ private Result countStateByProject(User loginUser, long projectCode, String startDate, String endDate,
+ TriFunction<Date, Date, Long[], List<ExecuteStatusCount>> instanceStateCounter) {
+ Result result = new Result();
if (projectCode != 0) {
Project project = projectMapper.queryByCode(projectCode);
- result = projectService.checkProjectAndAuth(loginUser, project, projectCode, PROJECT_OVERVIEW);
- if (result.get(Constants.STATUS) != Status.SUCCESS) {
+ Map<String, Object> checkResult =
+ projectService.checkProjectAndAuth(loginUser, project, projectCode, PROJECT_OVERVIEW);
Review Comment:
## User-controlled bypass of sensitive method
Sensitive method may not be executed depending on a [this condition](1), which flows from [user-controlled value](2).
Sensitive method may not be executed depending on a [this condition](1), which flows from [user-controlled value](3).
[Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/2342)
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/DataAnalysisService.java:
##########
@@ -63,23 +62,23 @@
* @param projectCode project code
* @return definition count data
*/
- Map<String, Object> countDefinitionByUser(User loginUser, long projectCode);
+ Result countDefinitionByUser(User loginUser, long projectCode);
/**
* statistical command status data
*
* @param loginUser login user
* @return command state count data
*/
- Map<String, Object> countCommandState(User loginUser);
+ Result countCommandState(User loginUser);
/**
* count queue state
*
* @param loginUser login user
* @return queue state count data
*/
- Map<String, Object> countQueueState(User loginUser);
+ Result countQueueState(User loginUser);
Review Comment:
## Useless parameter
The parameter 'loginUser' is never used.
[Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/2341)
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/DataAnalysisServiceImpl.java:
##########
@@ -205,22 +209,25 @@
* @return definition count data
*/
@Override
- public Map<String, Object> countDefinitionByUser(User loginUser, long projectCode) {
- Map<String, Object> result = new HashMap<>();
+ public Result countDefinitionByUser(User loginUser, long projectCode) {
+ Result result = new Result();
if (projectCode != 0) {
Project project = projectMapper.queryByCode(projectCode);
- result = projectService.checkProjectAndAuth(loginUser, project, projectCode, PROJECT_OVERVIEW);
- if (result.get(Constants.STATUS) != Status.SUCCESS) {
+ Map<String, Object> checkResult =
+ projectService.checkProjectAndAuth(loginUser, project, projectCode, PROJECT_OVERVIEW);
Review Comment:
## User-controlled bypass of sensitive method
Sensitive method may not be executed depending on a [this condition](1), which flows from [user-controlled value](2).
[Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/2343)
--
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] Zzih commented on pull request #12969: [Feature][Api] Refactor org.apache.dolphinscheduler.api.controller.DataAnalysisController
Posted by "Zzih (via GitHub)" <gi...@apache.org>.
Zzih commented on PR #12969:
URL: https://github.com/apache/dolphinscheduler/pull/12969#issuecomment-1413389070
@caishunfeng @SbloodyS Please help review that. please please please!!
--
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 #12969: [Feature][Api] Refactor org.apache.dolphinscheduler.api.controller.DataAnalysisController
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #12969:
URL: https://github.com/apache/dolphinscheduler/pull/12969#issuecomment-1326121610
# [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/12969?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 [#12969](https://codecov.io/gh/apache/dolphinscheduler/pull/12969?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ec2b13d) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/2f8f0952fb6e45b84437e9f651096858d856bd96?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2f8f095) will **increase** coverage by `0.06%`.
> The diff coverage is `98.63%`.
```diff
@@ Coverage Diff @@
## dev #12969 +/- ##
============================================
+ Coverage 39.32% 39.38% +0.06%
- Complexity 4252 4283 +31
============================================
Files 1061 1073 +12
Lines 40051 40113 +62
Branches 4604 4588 -16
============================================
+ Hits 15750 15800 +50
- Misses 22526 22539 +13
+ Partials 1775 1774 -1
```
| [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/12969?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...uler/api/service/impl/DataAnalysisServiceImpl.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12969/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvaW1wbC9EYXRhQW5hbHlzaXNTZXJ2aWNlSW1wbC5qYXZh) | `87.90% <96.42%> (+5.93%)` | :arrow_up: |
| [...heduler/api/controller/DataAnalysisController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12969/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvRGF0YUFuYWx5c2lzQ29udHJvbGxlci5qYXZh) | `100.00% <100.00%> (ø)` | |
| [...duler/api/controller/DataAnalysisV2Controller.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12969/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvRGF0YUFuYWx5c2lzVjJDb250cm9sbGVyLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...pi/dto/dataAnalysis/CommandStateCountResponse.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12969/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2R0by9kYXRhQW5hbHlzaXMvQ29tbWFuZFN0YXRlQ291bnRSZXNwb25zZS5qYXZh) | `100.00% <100.00%> (ø)` | |
| [...aAnalysis/ProcessDefinitionStateCountResponse.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12969/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2R0by9kYXRhQW5hbHlzaXMvUHJvY2Vzc0RlZmluaXRpb25TdGF0ZUNvdW50UmVzcG9uc2UuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...ataAnalysis/ProcessInstanceStateCountResponse.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12969/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2R0by9kYXRhQW5hbHlzaXMvUHJvY2Vzc0luc3RhbmNlU3RhdGVDb3VudFJlc3BvbnNlLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [.../api/dto/dataAnalysis/QueueStateCountResponse.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12969/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2R0by9kYXRhQW5hbHlzaXMvUXVldWVTdGF0ZUNvdW50UmVzcG9uc2UuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...r/api/dto/dataAnalysis/TaskStateCountResponse.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12969/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2R0by9kYXRhQW5hbHlzaXMvVGFza1N0YXRlQ291bnRSZXNwb25zZS5qYXZh) | `100.00% <100.00%> (ø)` | |
| [...er/master/dispatch/host/assign/RandomSelector.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12969/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9kaXNwYXRjaC9ob3N0L2Fzc2lnbi9SYW5kb21TZWxlY3Rvci5qYXZh) | `77.77% <0.00%> (-5.56%)` | :arrow_down: |
| [...eduler/api/dto/schedule/ScheduleCreateRequest.java](https://codecov.io/gh/apache/dolphinscheduler/pull/12969/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2R0by9zY2hlZHVsZS9TY2hlZHVsZUNyZWF0ZVJlcXVlc3QuamF2YQ==) | `83.33% <0.00%> (-2.39%)` | :arrow_down: |
| ... and [43 more](https://codecov.io/gh/apache/dolphinscheduler/pull/12969/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] Zzih commented on pull request #12969: [Feature][Api] Refactor org.apache.dolphinscheduler.api.controller.DataAnalysisController
Posted by GitBox <gi...@apache.org>.
Zzih commented on PR #12969:
URL: https://github.com/apache/dolphinscheduler/pull/12969#issuecomment-1326133351
@SbloodyS halo , Please help review that.
--
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] Zzih commented on pull request #12969: [Feature][Api] Refactor org.apache.dolphinscheduler.api.controller.DataAnalysisController
Posted by GitBox <gi...@apache.org>.
Zzih commented on PR #12969:
URL: https://github.com/apache/dolphinscheduler/pull/12969#issuecomment-1323498306
@SbloodyS Just accidentally closed the last PR.rebuild this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #12969: [Feature][Api] Refactor org.apache.dolphinscheduler.api.controller.DataAnalysisController
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12969:
URL: https://github.com/apache/dolphinscheduler/pull/12969#issuecomment-1326131777
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12969)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=CODE_SMELL) [55 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12969&metric=new_coverage&view=list) [98.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12969&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12969&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12969&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] Zzih closed pull request #12969: [Feature][Api] Refactor org.apache.dolphinscheduler.api.controller.DataAnalysisController
Posted by "Zzih (via GitHub)" <gi...@apache.org>.
Zzih closed pull request #12969: [Feature][Api] Refactor org.apache.dolphinscheduler.api.controller.DataAnalysisController
URL: https://github.com/apache/dolphinscheduler/pull/12969
--
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 #12969: [Feature][Api] Refactor org.apache.dolphinscheduler.api.controller.DataAnalysisController
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12969:
URL: https://github.com/apache/dolphinscheduler/pull/12969#issuecomment-1326128223
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12969)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=CODE_SMELL) [55 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12969&metric=new_coverage&view=list) [98.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12969&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12969&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12969&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] Zzih closed pull request #12969: [Feature][Api] Refactor org.apache.dolphinscheduler.api.controller.DataAnalysisController
Posted by "Zzih (via GitHub)" <gi...@apache.org>.
Zzih closed pull request #12969: [Feature][Api] Refactor org.apache.dolphinscheduler.api.controller.DataAnalysisController
URL: https://github.com/apache/dolphinscheduler/pull/12969
--
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] CalvinKirs commented on a diff in pull request #12969: [Feature][Api] Refactor org.apache.dolphinscheduler.api.controller.DataAnalysisController
Posted by "CalvinKirs (via GitHub)" <gi...@apache.org>.
CalvinKirs commented on code in PR #12969:
URL: https://github.com/apache/dolphinscheduler/pull/12969#discussion_r1094249844
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/DataAnalysisService.java:
##########
@@ -41,7 +42,7 @@ public interface DataAnalysisService {
* @param endDate end date
* @return task state count data
*/
- Map<String, Object> countTaskStateByProject(User loginUser, long projectCode, String startDate, String endDate);
+ Result countTaskStateByProject(User loginUser, long projectCode, String startDate, String endDate);
Review Comment:
`Result` should be encapsulated in `Collonter` layer, not `service` layer
--
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 #12969: [Feature][Api] Refactor org.apache.dolphinscheduler.api.controller.DataAnalysisController
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #12969:
URL: https://github.com/apache/dolphinscheduler/pull/12969#issuecomment-1413350844
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12969)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=CODE_SMELL) [57 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12969&metric=new_coverage&view=list) [89.4% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12969&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12969&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12969&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 #12969: [Feature][Api] Refactor org.apache.dolphinscheduler.api.controller.DataAnalysisController
Posted by "zhongjiajie (via GitHub)" <gi...@apache.org>.
zhongjiajie commented on PR #12969:
URL: https://github.com/apache/dolphinscheduler/pull/12969#issuecomment-1430838904
> Result should be encapsulated in Collonter layer, not service layer
I agree with Calvin, we should avoid returning simple object instead of results in service, especially since your patch is refactoring for our codebase, we should try to make it 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] davidzollo commented on a diff in pull request #12969: [Feature][Api] Refactor org.apache.dolphinscheduler.api.controller.DataAnalysisController
Posted by "davidzollo (via GitHub)" <gi...@apache.org>.
davidzollo commented on code in PR #12969:
URL: https://github.com/apache/dolphinscheduler/pull/12969#discussion_r1095574560
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/DataAnalysisService.java:
##########
@@ -41,7 +42,7 @@ public interface DataAnalysisService {
* @param endDate end date
* @return task state count data
*/
- Map<String, Object> countTaskStateByProject(User loginUser, long projectCode, String startDate, String endDate);
+ Result countTaskStateByProject(User loginUser, long projectCode, String startDate, String endDate);
Review Comment:
there are too many cases about `Result`, we can discuss where to put the `Result` later
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #12969: [Feature][Api] Refactor org.apache.dolphinscheduler.api.controller.DataAnalysisController
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #12969:
URL: https://github.com/apache/dolphinscheduler/pull/12969#issuecomment-1413346901
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12969)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=CODE_SMELL) [57 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12969&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12969&metric=new_coverage&view=list) [89.4% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12969&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12969&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12969&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] DarkAssassinator commented on pull request #12969: [Feature][Api] Refactor org.apache.dolphinscheduler.api.controller.DataAnalysisController
Posted by GitBox <gi...@apache.org>.
DarkAssassinator commented on PR #12969:
URL: https://github.com/apache/dolphinscheduler/pull/12969#issuecomment-1323691986
parameters -> DTO is a good refactor, but why all service return still keep `Result`? I think that `Result` is a bad partice for service impl. If failed, service just need throw `ServiceException`, no need update the `Result`, such as https://github.com/apache/dolphinscheduler/pull/12736/commits/f7caa6a92ea3e784db4ab0cbf96a00811a985c50. WDYT. cc @SbloodyS @caishunfeng
--
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] Zzih commented on a diff in pull request #12969: [Feature][Api] Refactor org.apache.dolphinscheduler.api.controller.DataAnalysisController
Posted by "Zzih (via GitHub)" <gi...@apache.org>.
Zzih commented on code in PR #12969:
URL: https://github.com/apache/dolphinscheduler/pull/12969#discussion_r1095302483
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/DataAnalysisService.java:
##########
@@ -41,7 +42,7 @@ public interface DataAnalysisService {
* @param endDate end date
* @return task state count data
*/
- Map<String, Object> countTaskStateByProject(User loginUser, long projectCode, String startDate, String endDate);
+ Result countTaskStateByProject(User loginUser, long projectCode, String startDate, String endDate);
Review Comment:
But in the interface v2(path:src/main/java/org/apache/dolphinscheduler/api/controller/v2), In most cases, Result be encapsulated in service layer
--
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] CalvinKirs commented on a diff in pull request #12969: [Feature][Api] Refactor org.apache.dolphinscheduler.api.controller.DataAnalysisController
Posted by "CalvinKirs (via GitHub)" <gi...@apache.org>.
CalvinKirs commented on code in PR #12969:
URL: https://github.com/apache/dolphinscheduler/pull/12969#discussion_r1095339025
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/DataAnalysisService.java:
##########
@@ -41,7 +42,7 @@ public interface DataAnalysisService {
* @param endDate end date
* @return task state count data
*/
- Map<String, Object> countTaskStateByProject(User loginUser, long projectCode, String startDate, String endDate);
+ Result countTaskStateByProject(User loginUser, long projectCode, String startDate, String endDate);
Review Comment:
We better be able to revisit this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] Zzih commented on pull request #12969: [Feature][Api] Refactor org.apache.dolphinscheduler.api.controller.DataAnalysisController
Posted by "Zzih (via GitHub)" <gi...@apache.org>.
Zzih commented on PR #12969:
URL: https://github.com/apache/dolphinscheduler/pull/12969#issuecomment-1413239397
https://github.com/apache/dolphinscheduler/pull/13031 This PR has been completed and merged, then I will close my 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