[fix-15907] Fix get remote shell exit code is incorrect by Zzih96 · Pull Request #15911 · apache/dolphinscheduler · GitHub
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[fix-15907] Fix get remote shell exit code is incorrect #15911

Merged
merged 7 commits into from
May 9, 2024
Merged

[fix-15907] Fix get remote shell exit code is incorrect #15911

merged 7 commits into from
May 9, 2024

Conversation

Zzih96
Copy link
Contributor

@Zzih96 Zzih96 commented Apr 25, 2024

Purpose of the pull request

fix #15907

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:

(or)

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

ruanwenjun
ruanwenjun requested changes Apr 25, 2024
View reviewed changes
Copy link
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

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

Please directly use StringUtils

@ruanwenjun ruanwenjun changed the title [fix-15907] fix getTaskExitCode [fix-15907] Fix get remote shell exit code is incorrect Apr 25, 2024
@ruanwenjun ruanwenjun added the bug Something isn't working label Apr 29, 2024
@ruanwenjun ruanwenjun added this to the 3.2.2 milestone Apr 29, 2024
@codecov-commenter
Copy link

codecov-commenter commented Apr 29, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 39.86%. Comparing base (ba5de75) to head (397a221).

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

Additional details and impacted files 
@@             Coverage Diff              @@
##                dev   #15911      +/-   ##
============================================
- Coverage     39.86%   39.86%   -0.01%     
+ Complexity     5060     5059       -1     
============================================
  Files          1369     1369              
  Lines         45635    45635              
  Branches       4869     4869              
============================================
- Hits          18193    18192       -1     
  Misses        25544    25544              
- Partials       1898     1899       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Zzih96 Zzih96 requested a review from ruanwenjun April 29, 2024 06:26
ruanwenjun
ruanwenjun approved these changes Apr 29, 2024
View reviewed changes
Copy link
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

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

LGTM

詹子恒 and others added 2 commits April 30, 2024 09:25
@ruanwenjun
./mvnw spotless:apply
a137b6e
@Zzih96 @ruanwenjun
Update dolphinscheduler-task-plugin/dolphinscheduler-task-remoteshell…
eb73f6d 
…/src/main/java/org/apache/dolphinscheduler/plugin/task/remoteshell/RemoteExecutor.java

Co-authored-by: Wenjun Ruan <wenjun@apache.org>
rickchengx
rickchengx approved these changes May 9, 2024
View reviewed changes
Copy link
Contributor

@rickchengx rickchengx left a comment

Choose a reason for hiding this comment

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

LGTM

@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented May 9, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@rickchengx rickchengx merged commit 5c569b7 into apache:dev May 9, 2024
62 checks passed
stoplyy added a commit to stoplyy/dolphinscheduler that referenced this pull request Sep 14, 2024
@stoplyy
follow apache#15911
f2e9159
@stoplyy
Copy link
Contributor

stoplyy commented Sep 21, 2024

It seems a new bug has been introduced.

The .trim() in the original code is necessary, otherwise the case of xxx-0\n cannot be handled.

image

@saowu
Copy link
Contributor

saowu commented Feb 12, 2025

It seems a new bug has been introduced.

The .trim() in the original code is necessary, otherwise the case of xxx-0\n cannot be handled.

image

such is the case

@ruanwenjun
Copy link
Member

@stoplyy Should be fixed by #16296

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ruanwenjun ruanwenjun ruanwenjun approved these changes

@rickchengx rickchengx rickchengx approved these changes

@caishunfeng caishunfeng Awaiting requested review from caishunfeng caishunfeng is a code owner

@SbloodyS SbloodyS Awaiting requested review from SbloodyS SbloodyS is a code owner

@zhuangchong zhuangchong Awaiting requested review from zhuangchong

Assignees

@Zzih96 Zzih96

Labels
3.2.2 backend bug Something isn't working priority:middle ready-to-merge
Projects
None yet
Milestone
3.2.2
7 participants
@Zzih96 @codecov-commenter @stoplyy @saowu @ruanwenjun @rickchengx @caishunfeng