[Feature-10219][EMR] EMR supports use <add-Steps> to add steps to an existing cluster #10657

guodongym · 2022-06-28T15:33:47Z

Add the ProgramType parameter to distinguish task types
EmrAddStepsTask supports Add-Steps
UI supports Add-Steps
EmrTask modify the name of the class to EmrJobFlowTask
add ERM Task abstract base class AbstractEmrTask
add testcase for EmrAddStepsTask
Modifying help Documents

close #10219

Purpose of the pull request

Brief change log

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

…existing cluster * Add the ProgramType parameter to distinguish task types * EmrAddStepsTask supports Add-Steps * UI supports Add-Steps * EmrTask modify the name of the class to EmrJobFlowTask * add ERM Task abstract base class AbstractEmrTask * add testcase for EmrAddStepsTask * Modifying help Documents

zhongjiajie · 2022-07-06T03:36:11Z

@guodongym Thanks to bring this up, and I personally hope it can merge soon

docs/docs/en/guide/task/emr.md

docs/docs/zh/guide/task/emr.md

...uler-task-emr/src/main/java/org/apache/dolphinscheduler/plugin/task/emr/AbstractEmrTask.java

zhongjiajie · 2022-07-06T03:51:44Z

BTW, doc conflict here

Amy0104

The front end part LGTM.

# Conflicts: # docs/docs/en/guide/task/emr.md

codecov-commenter · 2022-07-06T15:49:34Z

Codecov Report

Merging #10657 (a3bb733) into dev (8320490) will increase coverage by 0.03%.
The diff coverage is 62.13%.

❗ Current head a3bb733 differs from pull request most recent head a4c653e. Consider uploading reports for the commit a4c653e to get more accurate results

@@             Coverage Diff              @@
##                dev   #10657      +/-   ##
============================================
+ Coverage     40.64%   40.68%   +0.03%     
- Complexity     4820     4833      +13     
============================================
  Files           913      916       +3     
  Lines         36306    36375      +69     
  Branches       3993     3997       +4     
============================================
+ Hits          14756    14798      +42     
- Misses        20078    20106      +28     
+ Partials       1472     1471       -1

Impacted Files	Coverage Δ
...lphinscheduler/plugin/task/emr/EmrJobFlowTask.java	`82.35% <ø> (ø)`
...lphinscheduler/plugin/task/emr/EmrTaskChannel.java	`0.00% <0.00%> (ø)`
...phinscheduler/plugin/task/emr/AbstractEmrTask.java	`55.55% <55.55%> (ø)`
...phinscheduler/plugin/task/emr/EmrAddStepsTask.java	`67.79% <67.79%> (ø)`
...olphinscheduler/plugin/task/emr/EmrParameters.java	`76.92% <85.71%> (+5.49%)`	⬆️
.../dolphinscheduler/plugin/task/emr/ProgramType.java	`100.00% <100.00%> (ø)`
...erver/master/processor/queue/TaskEventService.java	`75.00% <0.00%> (-5.36%)`	⬇️
...org/apache/dolphinscheduler/remote/utils/Host.java	`43.47% <0.00%> (-2.18%)`	⬇️
...e/dolphinscheduler/remote/NettyRemotingClient.java	`51.38% <0.00%> (-1.39%)`	⬇️
...r/plugin/task/sqoop/parameter/SqoopParameters.java	`53.33% <0.00%> (-1.34%)`	⬇️
... and 8 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 8320490...a4c653e. Read the comment docs.

...eduler-task-emr/src/main/java/org/apache/dolphinscheduler/plugin/task/emr/EmrParameters.java

zhongjiajie · 2022-07-07T12:03:07Z

docs/docs/en/guide/task/emr.md

+| jobFlowDefineJson | JSON corresponding to the [RunJobFlowRequest](https://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/services/elasticmapreduce/model/RunJobFlowRequest.html) object, for details refer to [API_RunJobFlow_Examples](https://docs.aws.amazon.com/emr/latest/APIReference/API_RunJobFlow.html#API_RunJobFlow_Examples). |
+| stepsDefineJson | JSON corresponding to the [AddJobFlowStepsRequest](https://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/services/elasticmapreduce/model/AddJobFlowStepsRequest.html) object, for details refer to [API_AddJobFlowSteps_Examples](https://docs.aws.amazon.com/emr/latest/APIReference/API_AddJobFlowSteps.html#API_AddJobFlowSteps_Examples). |


could you migreate these two type as child or program type? just like Deployment mode in https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/en/guide/task/spark.md ?

These two parameters are not two types, but two elements that need to be filled in the form. I think they need to be specified separately

But does we can fill both two element in the single task?

BTW, can you add some screenshots for this task? I find it missing the screenshot in our docs, you can find some example in https://dolphinscheduler.apache.org/en-us/docs/dev/user_doc/guide/task/shell.html

But does we can fill both two element in the single task?

No, must to have a type to tell the difference, because the API that connects to AWS is completely different

BTW, can you add some screenshots for this task? I find it missing the screenshot in our docs, you can find some example in https://dolphinscheduler.apache.org/en-us/docs/dev/user_doc/guide/task/shell.html

yes, i can

But does we can fill both two element in the single task?

No, must to have a type to tell the difference, because the API that connects to AWS is completely different

If they can not fill the form in the same time, I think they should be in two different way here. but I do not have strong opinion

…existing cluster * Fix document conflicts * Fix code review comments * add some screenshots for this task

sonarcloud · 2022-07-09T15:34:55Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

58.7% Coverage
0.0% Duplication

zhongjiajie

LGTM, thanks

guodongym requested review from zhongjiajie, Tianqi-Dotes, songjianet, Amy0104, caishunfeng, SbloodyS and zhuangchong as code owners June 28, 2022 15:33

guodongym force-pushed the Feature-10219 branch from e80e1b8 to 7729210 Compare June 28, 2022 16:48

SbloodyS assigned guodongym Jun 29, 2022

SbloodyS added improvement make more easy to user or prompt friendly backend UI ui and front end related labels Jun 29, 2022

SbloodyS added this to the 3.1.0-alpha milestone Jun 29, 2022

guodongym force-pushed the Feature-10219 branch from 7729210 to 0dcad09 Compare June 29, 2022 02:27

zhongjiajie reviewed Jul 6, 2022

View reviewed changes

Amy0104 reviewed Jul 6, 2022

View reviewed changes

Merge branch 'dev' into Feature-10219

259d599

# Conflicts: # docs/docs/en/guide/task/emr.md

guodongym force-pushed the Feature-10219 branch from 60dd7e4 to a4c653e Compare July 6, 2022 15:30

guodongym closed this Jul 6, 2022

guodongym reopened this Jul 6, 2022

guodongym requested a review from zhongjiajie July 7, 2022 03:49

zhongjiajie reviewed Jul 7, 2022

View reviewed changes

guodongym force-pushed the Feature-10219 branch from a4c653e to e389512 Compare July 7, 2022 13:24

[Feature-10219][EMR] EMR supports use <add-Steps> to add steps to an …

8215b83

…existing cluster * Fix document conflicts * Fix code review comments * add some screenshots for this task

guodongym force-pushed the Feature-10219 branch from e389512 to 8215b83 Compare July 9, 2022 13:43

github-actions bot added the document label Jul 9, 2022

guodongym closed this Jul 9, 2022

guodongym reopened this Jul 9, 2022

guodongym closed this Jul 9, 2022

guodongym reopened this Jul 9, 2022

guodongym closed this Jul 9, 2022

guodongym reopened this Jul 9, 2022

guodongym requested a review from zhongjiajie July 9, 2022 15:42

zhongjiajie approved these changes Jul 10, 2022

View reviewed changes

zhongjiajie merged commit 8eaf5a2 into apache:dev Jul 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature-10219][EMR] EMR supports use <add-Steps> to add steps to an existing cluster #10657

[Feature-10219][EMR] EMR supports use <add-Steps> to add steps to an existing cluster #10657

guodongym commented Jun 28, 2022

zhongjiajie commented Jul 6, 2022

zhongjiajie commented Jul 6, 2022

Amy0104 left a comment

codecov-commenter commented Jul 6, 2022

zhongjiajie Jul 7, 2022

guodongym Jul 7, 2022

zhongjiajie Jul 7, 2022

zhongjiajie Jul 7, 2022

guodongym Jul 7, 2022

guodongym Jul 7, 2022

guodongym Jul 9, 2022

zhongjiajie Jul 10, 2022

sonarcloud bot commented Jul 9, 2022

zhongjiajie left a comment

		\| jobFlowDefineJson \| JSON corresponding to the [RunJobFlowRequest](https://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/services/elasticmapreduce/model/RunJobFlowRequest.html) object, for details refer to [API_RunJobFlow_Examples](https://docs.aws.amazon.com/emr/latest/APIReference/API_RunJobFlow.html#API_RunJobFlow_Examples). \|
		\| stepsDefineJson \| JSON corresponding to the [AddJobFlowStepsRequest](https://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/services/elasticmapreduce/model/AddJobFlowStepsRequest.html) object, for details refer to [API_AddJobFlowSteps_Examples](https://docs.aws.amazon.com/emr/latest/APIReference/API_AddJobFlowSteps.html#API_AddJobFlowSteps_Examples). \|

[Feature-10219][EMR] EMR supports use <add-Steps> to add steps to an existing cluster #10657

[Feature-10219][EMR] EMR supports use <add-Steps> to add steps to an existing cluster #10657

Conversation

guodongym commented Jun 28, 2022

Purpose of the pull request

Brief change log

Verify this pull request

zhongjiajie commented Jul 6, 2022

zhongjiajie commented Jul 6, 2022

Amy0104 left a comment

Choose a reason for hiding this comment

codecov-commenter commented Jul 6, 2022

Codecov Report

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

sonarcloud bot commented Jul 9, 2022

zhongjiajie left a comment

Choose a reason for hiding this comment