Skip to content

Conversation

@LiuCanWu
Copy link
Contributor

@LiuCanWu LiuCanWu commented Sep 12, 2023

Purpose of the pull request

fix #14729

Brief change log

1、The determinedYarnQueue method in the FlinkArgsUtils class and the switch statement of the doAddQueue method lack a break, adn I fix it.
2、 put the -Dyarn or -yqu parameter before -c, put the determinedYarnQueue method on line 284 before line 261

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:

(or)

If your pull request contain incompatible change, you should also add it to docs/docs/en/guide/upgrede/incompatible.md

@SbloodyS SbloodyS added bug Something isn't working first time contributor First-time contributor 3.1.x for 3.1.x version labels Sep 12, 2023
@SbloodyS SbloodyS added this to the 3.1.9 milestone Sep 12, 2023
@codecov-commenter
Copy link

codecov-commenter commented Sep 21, 2023

Codecov Report

Merging #14902 (9eab65e) into dev (2f2884f) will increase coverage by 0.00%.
The diff coverage is 66.66%.

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

@@            Coverage Diff            @@
##                dev   #14902   +/-   ##
=========================================
  Coverage     38.81%   38.81%           
- Complexity     4588     4590    +2     
=========================================
  Files          1234     1234           
  Lines         43385    43387    +2     
  Branches       4794     4792    -2     
=========================================
+ Hits          16841    16842    +1     
- Misses        24680    24681    +1     
  Partials       1864     1864           
Files Changed Coverage Δ
...hinscheduler/plugin/task/flink/FlinkArgsUtils.java 64.58% <66.66%> (-0.21%) ⬇️

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

Copy link
Member

@qingwli qingwli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@LiuCanWu LiuCanWu requested a review from qingwli September 21, 2023 06:24
@qingwli
Copy link
Member

qingwli commented Sep 21, 2023

Do you think this pr needs cherry-picked to 3.1.9? @zhuangchong

Copy link
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

66.7% 66.7% Coverage
0.0% 0.0% Duplication

@SbloodyS SbloodyS merged commit e748c2e into apache:dev Sep 21, 2023
zhongjiajie pushed a commit that referenced this pull request Oct 11, 2023
Co-authored-by: 刘阳 <[email protected]>
Co-authored-by: xiangzihao <[email protected]>
(cherry picked from commit e748c2e)
@zhuangchong zhuangchong added the release cherry-pick Mark this issue/PR had cherry-pick for release version label Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3.1.x for 3.1.x version backend bug Something isn't working first time contributor First-time contributor ready-to-merge release cherry-pick Mark this issue/PR had cherry-pick for release version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] [flink task] problem with the command generated by the flink task

5 participants