[Improvement-11880][Hivecli] Improve the UI of HiveCli #11882

rickchengx · 2022-09-09T08:02:42Z

Purpose of the pull request

close: #11880

if the user selects the type "FROM_FILE", he doesn't need to write sql statements in the HIVE SQL SCRIPT

Brief change log

Verify this pull request

manually tested

EricGao888

May I ask whether it is possible to make Resources mandatory if users choose FROM_FILE?

EricGao888 · 2022-09-09T09:05:47Z

May I ask whether it is possible to make Resources mandatory if users choose FROM_FILE?

BTW, we could also make HIVE SQL SCRIPT mandatory, I mean add a red star on UI for notice, if users choose FROM_SCRIPT.

EricGao888 · 2022-09-09T09:06:27Z

Overall, this is a good feature. It improves the interactions and I like it : )

rickchengx · 2022-09-13T03:54:17Z

May I ask whether it is possible to make Resources mandatory if users choose FROM_FILE?

BTW, we could also make HIVE SQL SCRIPT mandatory, I mean add a red star on UI for notice, if users choose FROM_SCRIPT.

Hi, @EricGao888 , thanks for your suggestions.

I have made HIVE SQL SCRIPT mandatary if users choose FROM_SCRIPT.

EricGao888 · 2022-09-13T03:57:18Z

May I ask whether it is possible to make Resources mandatory if users choose FROM_FILE?

BTW, we could also make HIVE SQL SCRIPT mandatory, I mean add a red star on UI for notice, if users choose FROM_SCRIPT.

Hi, @EricGao888 , thanks for your suggestions.

I have made HIVE SQL SCRIPT mandatary if users choose FROM_SCRIPT.

and made Resources mandatary if users choose FROM_FILE (also limit the number of resource file to 1).

Great job!

EricGao888

LGTM if CI passes.

rickchengx · 2022-09-23T03:02:04Z

Hi, @songjianet , @Amy0104, could you please help me this front-end problem?

It seems that resourcesRequired.value will lose the reactivity.

which means the user's click on the front end does not affect the required attribute in use-resources.ts

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

Lines 25 to 29 in 6eb1eb7

    
           export function useResources( 
        
             span: number | Ref<number> = 24, 
        
             required = false, 
        
             limit: number | Ref<number> = -1 
        
           ): IJsonItem {

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

Line 74 in 6eb1eb7

required: required,

codecov-commenter · 2022-09-23T05:39:40Z

Codecov Report

Merging #11882 (6eb1eb7) into dev (8a47785) will increase coverage by 0.11%.
The diff coverage is 49.36%.

❗ Current head 6eb1eb7 differs from pull request most recent head 40439f1. Consider uploading reports for the commit 40439f1 to get more accurate results

@@             Coverage Diff              @@
##                dev   #11882      +/-   ##
============================================
+ Coverage     38.57%   38.69%   +0.11%     
+ Complexity     4046     4010      -36     
============================================
  Files           995     1001       +6     
  Lines         36736    37422     +686     
  Branches       4280     4262      -18     
============================================
+ Hits          14171    14480     +309     
- Misses        20943    21295     +352     
- Partials       1622     1647      +25

Impacted Files	Coverage Δ
...olphinscheduler/api/audit/AuditPublishService.java	`34.61% <0.00%> (ø)`
...phinscheduler/api/controller/TenantController.java	`62.50% <ø> (ø)`
...heduler/api/dto/queue/QueueListPagingResponse.java	`0.00% <0.00%> (ø)`
...rg/apache/dolphinscheduler/api/k8s/K8sManager.java	`58.53% <0.00%> (ø)`
...lphinscheduler/api/permission/PermissionCheck.java	`0.00% <0.00%> (ø)`
...che/dolphinscheduler/api/python/PythonGateway.java	`17.22% <0.00%> (ø)`
...nscheduler/api/service/impl/DqRuleServiceImpl.java	`59.18% <0.00%> (ø)`
...api/service/impl/ProcessDefinitionServiceImpl.java	`31.38% <ø> (-0.65%)`	⬇️
...r/api/service/impl/ProcessInstanceServiceImpl.java	`57.71% <ø> (-3.15%)`	⬇️
...i/service/impl/ProcessTaskRelationServiceImpl.java	`21.32% <ø> (-1.13%)`	⬇️
... and 137 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Amy0104 · 2022-09-28T10:35:06Z

Hi, @songjianet , @Amy0104, could you please help me this front-end problem?

It seems that resourcesRequired.value will lose the reactivity.

which means the user's click on the front end does not affect the required attribute in use-resources.ts

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

Lines 25 to 29 in 6eb1eb7

export function useResources(

span: number | Ref<number> = 24,

required = false,

limit: number | Ref<number> = -1

): IJsonItem {

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

Line 74 in 6eb1eb7

required: required,

I'll check it.

Amy0104 · 2022-09-29T02:36:00Z

Maybe you can try to switch the useResource firstly, and then I try to support this by reactive required later. Is that ok? @rickchengx

rickchengx · 2022-09-29T02:56:34Z

Maybe you can try to switch the useResource firstly, and then I try to support this by reactive required later. Is that ok? @rickchengx

@Amy0104 Thanks a lot for the help. I have switched to the useResources().

Amy0104 · 2022-09-29T03:53:29Z

After this pr merged, I will fix this by #12202 .

sonarqubecloud · 2022-09-29T12:30:37Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

38.5% Coverage
0.0% Duplication

Amy0104

LGTM

Amy0104 · 2022-09-30T06:17:06Z

I have already set the required to be reactive.

rickchengx · 2022-10-12T08:14:51Z

I have already set the required to be reactive.

Thanks a lot, @Amy0104

I have made resources of hivecli task to be reactive in [Improvement-12335][ui] Make the resources to be reactive in hivecli task #12336

rickchengx requested review from Amy0104 and songjianet as code owners September 9, 2022 08:02

github-actions bot added the UI ui and front end related label Sep 9, 2022

SbloodyS requested a review from EricGao888 September 9, 2022 08:58

SbloodyS added the improvement make more easy to user or prompt friendly label Sep 9, 2022

SbloodyS assigned rickchengx Sep 9, 2022

SbloodyS added this to the 3.1.0 milestone Sep 9, 2022

EricGao888 reviewed Sep 9, 2022

View reviewed changes

rickchengx force-pushed the Improvement-11880 branch 3 times, most recently from c010c4b to 9bce22b Compare September 13, 2022 03:50

EricGao888 previously approved these changes Sep 13, 2022

View reviewed changes

rickchengx dismissed EricGao888’s stale review via 40439f1 September 22, 2022 04:06

rickchengx force-pushed the Improvement-11880 branch from 9bce22b to 40439f1 Compare September 22, 2022 04:06

caishunfeng modified the milestones: 3.1.0, 3.2.0 Sep 27, 2022

rickchengx force-pushed the Improvement-11880 branch from 40439f1 to f4a41f9 Compare September 29, 2022 02:54

[Improvement-11880][Hivecli] Improve the UI of HiveCli

4efd934

rickchengx force-pushed the Improvement-11880 branch from f4a41f9 to 4efd934 Compare September 29, 2022 07:06

Amy0104 approved these changes Sep 30, 2022

View reviewed changes

Amy0104 merged commit db9db94 into apache:dev Sep 30, 2022

Amy0104 mentioned this pull request Sep 30, 2022

[Improvement][UI] Set the required to be reactive in the task modal. #12225

Merged

rickchengx mentioned this pull request Oct 12, 2022

[Improvement][ui] Make the resources to be reactive in hivecli task #12335

Closed

3 tasks

xdu-chenrj pushed a commit to xdu-chenrj/dolphinscheduler that referenced this pull request Oct 13, 2022

[Improvement-11880][Hivecli] Improve the UI of HiveCli (apache#11882)

ac9072e

fuchanghai pushed a commit to fuchanghai/dolphinscheduler that referenced this pull request Nov 16, 2022

[Improvement-11880][Hivecli] Improve the UI of HiveCli (apache#11882)

f4c9493

[Improvement-11880][Hivecli] Improve the UI of HiveCli #11882

[Improvement-11880][Hivecli] Improve the UI of HiveCli #11882

Uh oh!

Conversation

rickchengx commented Sep 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose of the pull request

Brief change log

Verify this pull request

Uh oh!

EricGao888 left a comment

Choose a reason for hiding this comment

Uh oh!

EricGao888 commented Sep 9, 2022

Uh oh!

EricGao888 commented Sep 9, 2022

Uh oh!

rickchengx commented Sep 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EricGao888 commented Sep 13, 2022

Uh oh!

EricGao888 left a comment

Choose a reason for hiding this comment

Uh oh!

rickchengx commented Sep 23, 2022

Uh oh!

codecov-commenter commented Sep 23, 2022

Codecov Report

Uh oh!

Amy0104 commented Sep 28, 2022

Uh oh!

Amy0104 commented Sep 29, 2022

Uh oh!

rickchengx commented Sep 29, 2022

Uh oh!

Amy0104 commented Sep 29, 2022

Uh oh!

sonarqubecloud bot commented Sep 29, 2022

Uh oh!

Amy0104 left a comment

Choose a reason for hiding this comment

Uh oh!

Amy0104 commented Sep 30, 2022

Uh oh!

rickchengx commented Oct 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

rickchengx commented Sep 9, 2022 •

edited

Loading

rickchengx commented Sep 13, 2022 •

edited

Loading