batch copy or move process #2753 #2884

zixi0825 · 2020-06-03T04:02:00Z

What is the purpose of the pull request

This pull request adds batch copy or move process feature backend code #2753

Brief change log

Verify this pull request

codecov-commenter · 2020-06-03T07:00:25Z

Codecov Report

Merging #2884 into dev will increase coverage by 0.07%.
The diff coverage is 66.85%.

@@             Coverage Diff              @@
##                dev    #2884      +/-   ##
============================================
+ Coverage     34.15%   34.22%   +0.07%     
- Complexity     2422     2434      +12     
============================================
  Files           443      443              
  Lines         20637    20720      +83     
  Branches       2531     2544      +13     
============================================
+ Hits           7049     7092      +43     
- Misses        12926    12957      +31     
- Partials        662      671       +9

Impacted Files	Coverage Δ	Complexity Δ
...scheduler/api/controller/AlertGroupController.java	`5.26% <0.00%> (ø)`	`2.00 <0.00> (ø)`
...uler/api/controller/ProcessInstanceController.java	`4.00% <0.00%> (ø)`	`2.00 <0.00> (ø)`
...hinscheduler/api/controller/ProjectController.java	`5.88% <0.00%> (-0.57%)`	`2.00 <0.00> (ø)`
...olphinscheduler/api/service/DataSourceService.java	`0.61% <0.00%> (ø)`	`2.00 <0.00> (ø)`
...dolphinscheduler/api/service/SchedulerService.java	`24.78% <0.00%> (ø)`	`13.00 <0.00> (ø)`
...che/dolphinscheduler/common/utils/StringUtils.java	`75.00% <0.00%> (-10.72%)`	`10.00 <0.00> (ø)`
...e/dolphinscheduler/api/service/MonitorService.java	`13.63% <25.00%> (ø)`	`2.00 <1.00> (ø)`
.../dolphinscheduler/api/service/ExecutorService.java	`33.90% <33.33%> (ø)`	`19.00 <4.00> (ø)`
...cheduler/api/service/ProcessDefinitionService.java	`59.71% <53.75%> (-1.16%)`	`85.00 <34.00> (+10.00)`	⬇️
...phinscheduler/api/service/DataAnalysisService.java	`59.75% <75.00%> (ø)`	`14.00 <9.00> (ø)`
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b54c76...a4428dd. Read the comment docs.

break60 · 2020-06-10T02:29:56Z

Please solve ut

...pi/src/main/java/org/apache/dolphinscheduler/api/controller/ProcessDefinitionController.java

dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/enums/Status.java

...uler-api/src/main/java/org/apache/dolphinscheduler/api/service/ProcessDefinitionService.java

wen-hemin · 2020-06-11T06:12:56Z

After confirming the move workflow, does the modification on the workflow instance list page have an impact?
Confirm there are no file changes on the front-end page ?

gabrywu · 2020-06-13T12:56:49Z

@zixi0825 I think Rubik-W has already gave some good suggestions, can you reply to him ?

zixi0825 · 2020-06-15T08:00:38Z

After confirming the move workflow, does the modification on the workflow instance list page have an impact?

Confirm there are no file changes on the front-end page ?

to 1 : I will add the instance linkage after move workflow
to 2：The modification of the front-end will be completed by @break60

…duler into batch_copy_or_move_process

samz406 · 2020-06-23T14:28:13Z

...uler-api/src/main/java/org/apache/dolphinscheduler/api/service/ProcessDefinitionService.java

+                                                          String projectName,
+                                                          String processDefinitionIds,
+                                                          int targetProjectId, boolean isCopy){
+        Map<String, Object> result = new HashMap<>(5);


Hashmap does not recommend the initial value odd，No need to initialize

The initial value of HashMap in all Controller and Service in DolphinScheduler is 5, so if you want to modify it, then it is best to unify all changes after discussion

————————————————————————————————————————————
在DolphinScheduler中所有的Controller和Service中HashMap的初始值都为5，所以如果要修改的话，那么最好经过讨论以后，统一全部修改

This was discussed before, you can send an email and change it in a unified way
这个之前讨论过，你可以发个邮件，统一改了

...uler-api/src/main/java/org/apache/dolphinscheduler/api/service/ProcessDefinitionService.java

...rc/test/java/org/apache/dolphinscheduler/api/controller/ProcessDefinitionControllerTest.java

...uler-api/src/main/java/org/apache/dolphinscheduler/api/service/ProcessDefinitionService.java

...-api/src/test/java/org/apache/dolphinscheduler/api/service/ProcessDefinitionServiceTest.java

...rc/test/java/org/apache/dolphinscheduler/api/controller/ProcessDefinitionControllerTest.java

samz406 · 2020-06-23T14:57:24Z

...uler-api/src/main/java/org/apache/dolphinscheduler/api/service/ProcessDefinitionService.java

+                Map<String, Object> copyProcessDefinitionResult =
+                        copyProcessDefinition(loginUser,Integer.valueOf(processDefinitionId),targetProject);
+                if (!Status.SUCCESS.equals(copyProcessDefinitionResult.get(Constants.STATUS))) {
+                    failedIdList.add((String) copyProcessDefinitionResult.get(Constants.MSG));


i think failedIdList should store the id, but there added may be a failed description statement not the id

In my opinion, simply returning the id does not fully express the reason for the failure, I designed it to return a more complete cause of the error

在我看来，仅仅返回id并不能完全表达失败的原因，我这么设计是为了返回更加完整的错误原因

I have two questions:
1 Follow your current design. There may be a splicing return. Part of the error description and part of the id situation are not clear.
2 I think that the id should not be returned here, the process name should be returned, and the front end should be prompted, which of the several process errors is ok. If the id is returned, the front-end operator does not understand which process problem these id refers to

我有两点疑问，
1 按照你现在设计。有可能会出现拼接返回一部分是错误描述，一部分是id情况 ,这样描述不清晰了。
2 我觉得这里不应该返回id，应该返回流程名称。给前端提示，哪个几个流程错误就ok了，如果返回id，前端操作人员也不理解这些id指的是哪个流程问题。你觉得了

I have changed it to the name of the workflow that returned the error. If the error information is returned, it cannot be ruled out that the information is incomplete

我已经将其修改为返回出错的工作流名称了，返回错误描述的话确实不能排除信息不完整的情况

例子：如果工作流不存在的话，返回的信息如下：
从project_test1复制工作流到project_test1错误 : 47[null]

samz406 · 2020-06-23T15:05:24Z

dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ProjectService.java

@@ -43,7 +43,7 @@
 *HttpTask./


Modify note description

Makes no sense, I changed it back

samz406 · 2020-06-23T15:21:08Z

...cheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ProjectController.java

+    @ApiException(QUERY_USER_CREATED_PROJECT_ERROR)
+    public Result queryProjectCreatedByUser(@ApiIgnore @RequestAttribute(value = Constants.SESSION_USER) User loginUser) {
+        logger.info("login user {}, query authorized project by user id: {}.", StringUtils.replaceNRTtoUnderline(loginUser.getUserName()), StringUtils.replaceNRTtoUnderline(String.valueOf(loginUser.getId())));
+        Map<String, Object> result = projectService.queryProjectCreatedByUser(loginUser, loginUser.getId());


queryProjectCreatedByUser can define a parameter, you can not userid

The purpose of this function design is to get the project created by the logged-in user, so I think there is no problem using userid

————————————————————————————————————————————————————

这个函数设计的目的就是获取登录用户所创建的项目，所以我认为使用userid没有问题

queryProjectCreatedByUser 方法其实传loginUser 值就可以了啊。在方法里面直接loginUser.getId() 获取userid。不影响功能啊。

You're right

dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/BaseService.java

...uler-api/src/main/java/org/apache/dolphinscheduler/api/service/ProcessDefinitionService.java

yangyichao-mango · 2020-07-28T14:03:26Z

...pi/src/main/java/org/apache/dolphinscheduler/api/controller/ProcessDefinitionController.java

    })
-    @PostMapping(value = "/copy")
+    @PostMapping(value = "/copy-or-move")


Hi,
Will it be better to split copy and move to two api for single responsibility?

...pi/src/main/java/org/apache/dolphinscheduler/api/controller/ProcessDefinitionController.java

yangyichao-mango · 2020-07-28T14:06:50Z

...pi/src/main/java/org/apache/dolphinscheduler/api/controller/ProcessDefinitionController.java

+                                             @ApiParam(name = "projectName", value = "PROJECT_NAME", required = true) @PathVariable String projectName,
+                                             @RequestParam("processId") Integer processId


Please revert the changes about blank, thx a lot, it is suggested to add the blank according to the checkstyle.xml, you can configure the checkstyle.xml[1] to your ide.

[1] https://github.com/apache/incubator-dolphinscheduler/blob/dev/style/checkstyle.xml

…duler into batch_copy_or_move_process

…825/incubator-dolphinscheduler into batch_copy_or_move_process

davidzollo

+1
good job
please one PR solve one issue next time, thx

gabrywu · 2020-08-07T06:03:58Z

Hi , @samz406 @yangyichao-mango , could this PR be merged?

yangyichao-mango · 2020-08-07T10:52:36Z

Hi , @samz406 @yangyichao-mango , could this PR be merged?

I think split copy and move to two api will be better, but at present it LGTM, we can improve it later.

zixi0825 · 2020-08-07T11:09:43Z

Hi , @samz406 @yangyichao-mango , could this PR be merged?

I think split copy and move to two api will be better, but at present it LGTM, we can improve it later.

i will split it next week

gabrywu · 2020-08-10T06:33:50Z

Hi , @samz406 @yangyichao-mango , could this PR be merged?

I think split copy and move to two api will be better, but at present it LGTM, we can improve it later.

i will split it next week

@yangyichao-mango @zixi0825 split in this PR?

zixi0825 · 2020-08-11T03:14:26Z

Hi , @samz406 @yangyichao-mango , could this PR be merged?

I think split copy and move to two api will be better, but at present it LGTM, we can improve it later.

i will split it next week

@yangyichao-mango @zixi0825 split in this PR?

yes

gabrywu · 2020-08-11T05:51:58Z

@zixi0825 Please resolve the conflicting files, then PR can be merged

sonarcloud · 2020-08-11T08:17:15Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
6 Code Smells

67.9% Coverage
0.0% Duplication

yangyichao-mango

LGTM.

sunchaohe added 3 commits June 3, 2020 11:57

batch copy or move process

53c2ba2

batch copy or move process

9a4ad29

batch copy or move process

247503a

sunchaohe and others added 5 commits June 3, 2020 16:41

batch copy or move process

55a21cd

batch copy or move process

309fa0c

batch copy or move process

4694500

batch copy or move process

bb41a4f

Merge branch 'dev' into batch_copy_or_move_process

2b71e4e

Merge branch 'dev' into batch_copy_or_move_process

a273966

wen-hemin requested changes Jun 11, 2020

View reviewed changes

sunchaohe and others added 12 commits June 18, 2020 18:45

batch copy or move process sovle code smell

97815cd

Merge branch 'dev' of https://github.com/apache/incubator-dolphinsche…

79504d9

…duler into batch_copy_or_move_process

batch copy or move process sovle code smell

19794ca

batch copy or move process sovle code smell

1ed2361

batch copy or move process sovle code smell

5ede5e3

batch copy or move process sovle code smell

0431b9d

batch copy or move process sovle code smell

c18e98b

batch copy or move process sovle code smell

81c9d08

batch copy or move process sovle code smell

6ac6186

Merge branch 'dev' into batch_copy_or_move_process

8317b00

Merge branch 'dev' into batch_copy_or_move_process

32695ec

Merge branch 'dev' into batch_copy_or_move_process

d49376b

samz406 reviewed Jun 23, 2020

View reviewed changes

zixi0825 and others added 2 commits June 24, 2020 14:11

Merge branch 'dev' into batch_copy_or_move_process

b7187b8

batch copy or move process

25ae29b

wen-hemin self-requested a review July 28, 2020 06:21

yangyichao-mango reviewed Jul 28, 2020

View reviewed changes

dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/BaseService.java Outdated Show resolved Hide resolved

yangyichao-mango reviewed Jul 28, 2020

View reviewed changes

dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/BaseService.java Outdated Show resolved Hide resolved

yangyichao-mango reviewed Jul 28, 2020

View reviewed changes

...uler-api/src/main/java/org/apache/dolphinscheduler/api/service/ProcessDefinitionService.java Outdated Show resolved Hide resolved

yangyichao-mango reviewed Jul 28, 2020

View reviewed changes

...pi/src/main/java/org/apache/dolphinscheduler/api/controller/ProcessDefinitionController.java Outdated Show resolved Hide resolved

yangyichao-mango reviewed Jul 28, 2020

View reviewed changes

sunchaohe and others added 4 commits July 29, 2020 10:23

revert blank

123e272

Merge branch 'dev' of https://github.com/apache/incubator-dolphinsche…

a86b4e1

…duler into batch_copy_or_move_process

Merge branch 'batch_copy_or_move_process' of https://github.com/zixi0…

23897a4

…825/incubator-dolphinscheduler into batch_copy_or_move_process

Merge branch 'dev' into batch_copy_or_move_process

2153681

davidzollo approved these changes Aug 1, 2020

View reviewed changes

sunchaohe added 5 commits August 11, 2020 15:01

resolve conflicts

47edf92

resolve conflicts

240b712

resolve conflicts

a8b7c95

resolve ut error

002b2e6

resolve sonar error

ee4f85e

yangyichao-mango approved these changes Aug 12, 2020

View reviewed changes

davidzollo added this to the 1.4.0 milestone Aug 13, 2020

davidzollo merged commit 46ab86d into apache:dev Aug 13, 2020

davidzollo mentioned this pull request Aug 13, 2020

[Feature] Copy or move process to other projects #2753

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

batch copy or move process #2753 #2884

batch copy or move process #2753 #2884

zixi0825 commented Jun 3, 2020

codecov-commenter commented Jun 3, 2020 •

edited

Loading

break60 commented Jun 10, 2020

wen-hemin commented Jun 11, 2020

gabrywu commented Jun 13, 2020

zixi0825 commented Jun 15, 2020

samz406 Jun 23, 2020

zixi0825 Jun 24, 2020 •

edited

Loading

samz406 Jun 24, 2020

zixi0825 Jun 28, 2020

samz406 Jun 23, 2020

zixi0825 Jun 24, 2020 •

edited

Loading

samz406 Jun 24, 2020

zixi0825 Jun 24, 2020

samz406 Jun 23, 2020

zixi0825 Jun 24, 2020

samz406 Jun 23, 2020

zixi0825 Jun 24, 2020

samz406 Jun 24, 2020

zixi0825 Jun 24, 2020

yangyichao-mango Jul 28, 2020

yangyichao-mango Jul 28, 2020

davidzollo left a comment

gabrywu commented Aug 7, 2020

yangyichao-mango commented Aug 7, 2020

zixi0825 commented Aug 7, 2020

gabrywu commented Aug 10, 2020

zixi0825 commented Aug 11, 2020

gabrywu commented Aug 11, 2020

sonarcloud bot commented Aug 11, 2020

yangyichao-mango left a comment

		@ApiParam(name = "projectName", value = "PROJECT_NAME", required = true) @PathVariable String projectName,
		@RequestParam("processId") Integer processId

batch copy or move process #2753 #2884

batch copy or move process #2753 #2884

Conversation

zixi0825 commented Jun 3, 2020

What is the purpose of the pull request

Brief change log

Verify this pull request

codecov-commenter commented Jun 3, 2020 • edited Loading

Codecov Report

break60 commented Jun 10, 2020

wen-hemin commented Jun 11, 2020

gabrywu commented Jun 13, 2020

zixi0825 commented Jun 15, 2020

Choose a reason for hiding this comment

zixi0825 Jun 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zixi0825 Jun 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidzollo left a comment

Choose a reason for hiding this comment

gabrywu commented Aug 7, 2020

yangyichao-mango commented Aug 7, 2020

zixi0825 commented Aug 7, 2020

gabrywu commented Aug 10, 2020

zixi0825 commented Aug 11, 2020

gabrywu commented Aug 11, 2020

sonarcloud bot commented Aug 11, 2020

yangyichao-mango left a comment

Choose a reason for hiding this comment

codecov-commenter commented Jun 3, 2020 •

edited

Loading

zixi0825 Jun 24, 2020 •

edited

Loading

zixi0825 Jun 24, 2020 •

edited

Loading