[Feature- 9837][plugin/ui] support FlinkSQL Task #9840

Dannila · 2022-04-28T11:08:34Z

Purpose of the pull request

We want FlinkTask to have an editor to fill out flink sql.

Users can implement flink sql tasks or flink sql cdc tasks through the sql editor, which is convenient for users to perform data processing and ETL.

This change added tests and can be verified as follows:

Brief change log

ProgramType.class add sql type
FlinkTask.class and FlinkTask.class add main logical for support Spark sql command
FlinkConstants.class 和 FlinkParameters.class Common parameters for connecting Flink sql commands and methods for obtaining parameters
use-flink.ts adds some logic to support front-end pages supporting spark sql editing SQL

close #9837

Verify this pull request

This change added tests and can be verified as follows:

org.apache.dolphinscheduler.plugin.task.flink.FlinkTaskTest

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:

labbomb · 2022-04-29T01:32:38Z

@Amy0104 @devosend Take a look at this PR

SbloodyS · 2022-04-29T01:34:41Z

Hi @Dannila , can you please add this to the docs? Thanks.

codecov-commenter · 2022-04-29T01:49:56Z

Codecov Report

Merging #9840 (bbd92dd) into dev (8562f6a) will increase coverage by 0.36%.
The diff coverage is 66.88%.

@@             Coverage Diff              @@
##                dev    #9840      +/-   ##
============================================
+ Coverage     40.31%   40.67%   +0.36%     
- Complexity     4532     4576      +44     
============================================
  Files           835      834       -1     
  Lines         33750    33829      +79     
  Branches       3727     3745      +18     
============================================
+ Hits          13606    13761     +155     
+ Misses        18857    18749     -108     
- Partials       1287     1319      +32

Impacted Files	Coverage Δ
...hinscheduler/plugin/task/flink/FlinkConstants.java	`0.00% <ø> (ø)`
.../dolphinscheduler/plugin/task/flink/FlinkTask.java	`65.45% <65.51%> (+65.45%)`	⬆️
...inscheduler/plugin/task/flink/FlinkParameters.java	`98.24% <85.71%> (+80.59%)`	⬆️
...olphinscheduler/plugin/task/flink/ProgramType.java	`100.00% <100.00%> (+100.00%)`	⬆️
...org/apache/dolphinscheduler/remote/utils/Host.java	`37.77% <0.00%> (-2.23%)`	⬇️
...dolphinscheduler/remote/future/ResponseFuture.java	`81.96% <0.00%> (-1.64%)`	⬇️
...e/dolphinscheduler/remote/NettyRemotingClient.java	`52.11% <0.00%> (-1.41%)`	⬇️
...r/plugin/task/sqoop/parameter/SqoopParameters.java	`53.33% <0.00%> (-1.34%)`	⬇️
... and 1 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 8562f6a...bbd92dd. Read the comment docs.

...r-task-flink/src/main/java/org/apache/dolphinscheduler/plugin/task/flink/FlinkConstants.java

Dannila · 2022-04-29T02:05:00Z

你好@Dannila，你能把这个添加到文档中吗？谢谢。

ok, another pr will be mentioned later to add documents

devosend · 2022-04-29T02:34:29Z

dolphinscheduler-ui-next/src/views/projects/task/components/node/fields/use-flink.ts

@@ -14,15 +14,27 @@
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
-import { computed, ref } from 'vue'
+import { computed, ref , watchEffect} from 'vue'


Prettier format code should be used here.

WatchEffect is added here to monitor the value of programType and let flinkVersion change dynamically

I don't have a better way to modify it

I don't have a better way to modify it

this is a matter of code style. import { computed, ref, watchEffect} from 'vue'

You can install prettier plugin for editor or run pnpm run prettier

devosend · 2022-04-29T02:34:55Z

dolphinscheduler-ui-next/src/views/projects/task/components/node/fields/use-flink.ts

+    model.programType === 'SQL' ? 24 : 0
+  )
+
+  const flinkVersionOptions = computed(() =>


devosend · 2022-04-29T03:10:29Z

dolphinscheduler-ui-next/src/views/projects/task/components/node/fields/use-flink.ts

        validator(validate: any, value: string) {
-          if (model.programType !== 'PYTHON' && !value) {
+          if (model.programType !== 'PYTHON' && !value && model.programType !== 'SQL') {
            return new Error(t('project.node.main_class_tips'))
          }
        }
      }
    },
    useMainJar(model),


You should judge programType in useMainJar . It should not request queryResourceByProgramType when programType is SQL

When the user clicks the programType as sql on the front end, the mainClass will be removed, and the edit box will be displayed for the user to fill in the sql

When the user clicks the programType as sql on the front end, the mainClass will be removed, and the edit box will be displayed for the user to fill in the sql

But it still call queryResourceByProgramType func and send a request to the backend.

@Dannila modify use-main-jar.ts watch function

SbloodyS · 2022-04-29T03:35:44Z

ok, another pr will be mentioned later to add documents

Please also add documents in this PR. Thanks.

Dannila · 2022-04-29T06:07:03Z

还请在此 PR 中添加文档。谢谢。

Quindi aggiungerò il documento ora

davidzollo · 2022-05-04T13:35:51Z

ok, another pr will be mentioned later to add documents

Please also add documents in this PR. Thanks.

good job, supporting Flink sql is big feature, By the way, the doc is very important, the PR with relevant doc will keep the code and documentation as consistent as possible

caishunfeng · 2022-05-05T02:41:36Z

Well done, I had approved to run CI.

caishunfeng · 2022-05-05T02:43:40Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs 0 Vulnerabilities 2 Security Hotspots 5 Code Smells

71.8% Coverage 2.2% Duplication

BTW, please check the https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=9840&resolved=false&types=SECURITY_HOTSPOT

Dannila · 2022-05-05T06:04:28Z

@zhuangchong Excuse me, do you think this PR needs to be further optimized?

SbloodyS · 2022-05-06T07:36:27Z

@Dannila Please resolve conflicts.

Dannila · 2022-05-06T08:04:06Z

@Dannila Please resolve conflicts.

OK, I'll fix it in the next commit.

songjianet · 2022-05-06T09:06:15Z

Please rebase the latest code, we removed the old UI.

Dannila · 2022-05-06T09:11:24Z

Please rebase the latest code, we removed the old UI.

Yes, in the latest commit, I have rebase the latest code.

songjianet · 2022-05-06T09:13:34Z

Please rebase the latest code, we removed the old UI.

Yes, in the latest commit, I have rebase the latest code.

It may be an operation problem. We removed the old UI and renamed UI-Next to UI, so from the current point of view, the operation on your side has not been successful.

Dannila · 2022-05-07T01:55:50Z

请rebase最新的代码，我们删除旧的UI。

是的，在最新的提交中，我有rebase最新的代码。

可能是操作。我们取消了旧的UI改写，所以从目前UI-Next来看UI，你的这个操作没有成功。

Excuse me, is this operation successful in the latest commit?

songjianet · 2022-05-07T01:57:26Z

yes, will review later.

Dannila · 2022-05-07T01:58:11Z

yes, will review later.

Gratitude！

docs/docs/zh/guide/task/flink.md

SbloodyS

The backend part LGTM. PTAL @zhongjiajie

docs/docs/en/guide/task/flink.md

docs/docs/zh/guide/task/flink.md

devosend · 2022-05-09T02:52:21Z

To ensure the quality and style of the front-end code, you should run pnpm run prettier to format the front-end code. @Dannila

zhongjiajie · 2022-05-09T02:57:00Z

could we add it to CI? when could not formatted will fail our CI @devosend ?

zhongjiajie

Other LGTM

docs/docs/en/guide/task/flink.md

docs/docs/zh/guide/task/flink.md

zhongjiajie · 2022-05-09T03:01:21Z

we should have frontend get at least one approval from our frontend developer

Dannila · 2022-05-09T03:03:16Z

To ensure the quality and style of the front-end code, you should run pnpm run prettier to format the front-end code. @Dannila

Sorry, I don't know much about front-end, I don't know much about this should run pnpm run prettier to format front-end code

devosend

The front end part LGTM

Dannila · 2022-05-09T10:48:05Z

The front end part LGTM

Greatful

sonarcloud · 2022-05-09T10:51:22Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
2 Security Hotspots
5 Code Smells

74.9% Coverage
2.2% Duplication

SbloodyS · 2022-05-09T10:52:01Z

Thanks for your contribution. This is a good beginning. Considering that it's your first contribution, I think we can get deep communication, you can contact me through mail or add WeChat(SbloodyS), when mail or added, please tell me who you are. I think I can help to familiar with the Dolphinscheduler if you meet with problems.

zhuangchong added the first time contributor First-time contributor label Apr 29, 2022

SbloodyS added UI ui and front end related backend miss:docs missing documents in PR labels Apr 29, 2022

caishunfeng requested review from zhuangchong and devosend April 29, 2022 01:36

SbloodyS reviewed Apr 29, 2022

View reviewed changes

...r-task-flink/src/main/java/org/apache/dolphinscheduler/plugin/task/flink/FlinkConstants.java Show resolved Hide resolved

devosend reviewed Apr 29, 2022

View reviewed changes

davidzollo removed the miss:docs missing documents in PR label May 4, 2022

Dannila added 6 commits May 6, 2022 18:01

flink_sql

628173b

'[refactor]flinksql'

084ad79

'[refactor]flinksql'

50a88f0

'[refactor]flinksql'

6498aa3

'[refactor]flinksql'

8822063

'[refactor]flinksql'

413bd69

Dannila added 3 commits May 6, 2022 18:01

[refactor] flink sql

391d102

[refactor] flink sql

60ef0a0

[refactor] flink sql

6fe7539

SbloodyS reviewed May 7, 2022

View reviewed changes

docs/docs/zh/guide/task/flink.md Outdated Show resolved Hide resolved

SbloodyS reviewed May 7, 2022

View reviewed changes

Dannila added 2 commits May 7, 2022 10:37

[refactor] flink sql

e21eb27

[refactor] flink sql

03018de

SbloodyS reviewed May 7, 2022

View reviewed changes

docs/docs/en/guide/task/flink.md Outdated Show resolved Hide resolved

[refactor] flink sql

7119d6e

SbloodyS reviewed May 7, 2022

View reviewed changes

docs/docs/zh/guide/task/flink.md Outdated Show resolved Hide resolved

[refactor] flink sql

bbd92dd

zhuangchong previously approved these changes May 9, 2022

View reviewed changes

zhongjiajie reviewed May 9, 2022

View reviewed changes

docs/docs/en/guide/task/flink.md Outdated Show resolved Hide resolved

docs/docs/zh/guide/task/flink.md Outdated Show resolved Hide resolved

Update docs/docs/en/guide/task/flink.md

3f129ed

zhongjiajie dismissed zhuangchong’s stale review via 3f129ed May 9, 2022 02:59

Update docs/docs/zh/guide/task/flink.md

f49f2d0

prettier front-end code

2d85260

devosend approved these changes May 9, 2022

View reviewed changes

SbloodyS merged commit 2d36449 into apache:dev May 9, 2022

songjianet added this to the 3.1.0 milestone May 18, 2022

[Feature- 9837][plugin/ui] support FlinkSQL Task #9840

[Feature- 9837][plugin/ui] support FlinkSQL Task #9840

Conversation

Dannila commented Apr 28, 2022 • edited by SbloodyS Loading

Purpose of the pull request

Brief change log

Verify this pull request

labbomb commented Apr 29, 2022

SbloodyS commented Apr 29, 2022

codecov-commenter commented Apr 29, 2022 • edited Loading

Codecov Report

Dannila commented Apr 29, 2022 • 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

SbloodyS commented Apr 29, 2022

Dannila commented Apr 29, 2022

davidzollo commented May 4, 2022

caishunfeng commented May 5, 2022

caishunfeng commented May 5, 2022

Dannila commented May 5, 2022

SbloodyS commented May 6, 2022

Dannila commented May 6, 2022

songjianet commented May 6, 2022

Dannila commented May 6, 2022

songjianet commented May 6, 2022

Dannila commented May 7, 2022

songjianet commented May 7, 2022

Dannila commented May 7, 2022

SbloodyS left a comment

Choose a reason for hiding this comment

devosend commented May 9, 2022

zhongjiajie commented May 9, 2022

zhongjiajie left a comment

Choose a reason for hiding this comment

zhongjiajie commented May 9, 2022

Dannila commented May 9, 2022

devosend left a comment

Choose a reason for hiding this comment

Dannila commented May 9, 2022

sonarcloud bot commented May 9, 2022

SbloodyS commented May 9, 2022

Dannila commented Apr 28, 2022 •

edited by SbloodyS

Loading

codecov-commenter commented Apr 29, 2022 •

edited

Loading

Dannila commented Apr 29, 2022 •

edited

Loading