Skip to content

PulsarAdmin: Fix last exit code storage#12581

Merged
codelipenghui merged 3 commits into
apache:masterfrom
thomasleplus:master
Nov 6, 2021
Merged

PulsarAdmin: Fix last exit code storage#12581
codelipenghui merged 3 commits into
apache:masterfrom
thomasleplus:master

Conversation

@thomasleplus

@thomasleplus thomasleplus commented Nov 2, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

This line was actually assigning the method argument to itself instead of setting the static field of the same name.

Motivation

Fix a simple mistake.

Modifications

Trivial change.

Verifying this change

  • Make sure that the change passes the CI checks.

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): no
  • The public API: no
  • The schema: no
  • The default values of configurations: no
  • The wire protocol: no
  • The rest endpoints: no
  • The admin cli options: no
  • Anything that affects deployment: no

Documentation

Check the box below and label this PR (if you have committer privilege).

Need to update docs?

  • doc-required
  • no-need-doc This is a trivial change that does not impact the API or schema.
  • doc

@github-actions

github-actions Bot commented Nov 2, 2021

Copy link
Copy Markdown

@thomasleplus:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

@thomasleplus thomasleplus mentioned this pull request Nov 2, 2021
Closed
4 tasks
@github-actions

github-actions Bot commented Nov 2, 2021

Copy link
Copy Markdown

@thomasleplus:Thanks for providing doc info!

@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Nov 2, 2021
@github-actions

github-actions Bot commented Nov 2, 2021

Copy link
Copy Markdown

@thomasleplus:Thanks for providing doc info!

@yuruguo

yuruguo commented Nov 2, 2021

Copy link
Copy Markdown
Contributor

Sorry, the PR I submitted has covered this problem. @315157973 @thomasleplus

@thomasleplus

thomasleplus commented Nov 2, 2021

Copy link
Copy Markdown
Contributor Author

Sorry, the PR I submitted has covered this problem. @315157973 @thomasleplus

No problem at all. I'm just glad the issue is resolved.

@315157973

315157973 commented Nov 2, 2021

Copy link
Copy Markdown
Contributor

Sorry, the PR I submitted has covered this problem. @315157973 @thomasleplus

This PR does not conflict with your PR, this PR needs to be merged into the 2.8.3 branch

@315157973

315157973 commented Nov 2, 2021

Copy link
Copy Markdown
Contributor

@thomasleplus Please take a look the unit test

nicoloboschi
nicoloboschi approved these changes Nov 2, 2021
View reviewed changes

@nicoloboschi nicoloboschi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IIUC this is only related to tests, not the command line usage. We need to explicit it in the PR title

yuruguo
yuruguo requested changes Nov 2, 2021
View reviewed changes

@yuruguo yuruguo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please pay attention to related tests

pulsar/pulsar-client-tools/src/test/java/org/apache/pulsar/admin/cli/TestRunMain.java

Line 28 in 3eec833

public class TestRunMain {

@thomasleplus

thomasleplus commented Nov 2, 2021

Copy link
Copy Markdown
Contributor Author

Sorry I thought it may be a flaky test. I pushed a fix.

yuruguo
yuruguo requested changes Nov 3, 2021
View reviewed changes

@yuruguo yuruguo left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please pay attention to runMainNoArguments() test

pulsar/pulsar-client-tools/src/test/java/org/apache/pulsar/admin/cli/TestRunMain.java

Line 34 in 3eec833

assertEquals(PulsarAdminTool.getLastExitCode(), 0);

0 -> 1

@yuruguo yuruguo mentioned this pull request Nov 3, 2021
Closed
1 task
@codelipenghui
codelipenghui merged commit d357cec into apache:master Nov 6, 2021
codelipenghui pushed a commit that referenced this pull request Nov 7, 2021
@thomasleplus @codelipenghui
PulsarAdmin: Fix last exit code storage (#12581)
4e4f49d 
(cherry picked from commit d357cec)
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Nov 7, 2021
zeo1995 pushed a commit to zeo1995/pulsar that referenced this pull request Nov 7, 2021
Yan Zhang
Merge branch 'up/master' into website/branch-2.7.2-chapter-5
c84b320 
* up/master: (55 commits)
  [broker] remove useless method "PersistentTopic#getPersistentTopic" (apache#12655)
  [Python Schema] Python schema support custom Avro configurations for Enum type (apache#12642)
  Allow to configure different implementations for Pulsar functions state store (apache#12646)
  Remove replicator global test from the quarantine group (apache#12648)
  [Java Client] Remove invalid call to Thread.currentThread().interrupt(); (apache#12652)
  k8s runtime: force deletion to avoid hung function worker during connector restart (apache#12504)
  [Broker] Optimize exception information for schemas (apache#12647)
  Close Zk database on unit tests (apache#12649)
  Fix call sync method in an async callback when enabling geo replicator. (apache#12590)
  [pulsar-broker] Add git branch information for PulsarVersion (apache#12541)
  PulsarAdmin: Fix last exit code storage (apache#12581)
  Add @test annotation to test methods (apache#12640)
  Upgrade debezium to 1.7.1 (apache#12644)
  [ML] Avoid passing OpAddEntry across a thread boundary in asyncAddEntry (apache#12606)
  [Functions] Prevent NPE while stopping a non started Pulsar LogAppender (apache#12643)
  Update io-debezium-source.md (apache#12638)
  Add missing cmds on pulsar-admin document page (apache#12634)
  Clean up the metadata of the non-persistent partitioned topics. (apache#12550)
  modify check waitingForPingResponse with volatile (apache#12615)
  [pulsar-admin] Check backlog quota policy for namespace (apache#12512)
  ...
zeo1995 pushed a commit to zeo1995/pulsar that referenced this pull request Nov 7, 2021
Yan Zhang
Merge branch 'up/master' into website/branch-2.7.2-chapter-6
497e20e 
* up/master: (55 commits)
  [broker] remove useless method "PersistentTopic#getPersistentTopic" (apache#12655)
  [Python Schema] Python schema support custom Avro configurations for Enum type (apache#12642)
  Allow to configure different implementations for Pulsar functions state store (apache#12646)
  Remove replicator global test from the quarantine group (apache#12648)
  [Java Client] Remove invalid call to Thread.currentThread().interrupt(); (apache#12652)
  k8s runtime: force deletion to avoid hung function worker during connector restart (apache#12504)
  [Broker] Optimize exception information for schemas (apache#12647)
  Close Zk database on unit tests (apache#12649)
  Fix call sync method in an async callback when enabling geo replicator. (apache#12590)
  [pulsar-broker] Add git branch information for PulsarVersion (apache#12541)
  PulsarAdmin: Fix last exit code storage (apache#12581)
  Add @test annotation to test methods (apache#12640)
  Upgrade debezium to 1.7.1 (apache#12644)
  [ML] Avoid passing OpAddEntry across a thread boundary in asyncAddEntry (apache#12606)
  [Functions] Prevent NPE while stopping a non started Pulsar LogAppender (apache#12643)
  Update io-debezium-source.md (apache#12638)
  Add missing cmds on pulsar-admin document page (apache#12634)
  Clean up the metadata of the non-persistent partitioned topics. (apache#12550)
  modify check waitingForPingResponse with volatile (apache#12615)
  [pulsar-admin] Check backlog quota policy for namespace (apache#12512)
  ...
@rdhabalia rdhabalia mentioned this pull request Nov 8, 2021
Merged
eolivelli pushed a commit to eolivelli/pulsar that referenced this pull request Nov 29, 2021
@thomasleplus @eolivelli
PulsarAdmin: Fix last exit code storage (apache#12581)
e29c740
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@315157973 315157973 315157973 approved these changes
@codelipenghui codelipenghui codelipenghui approved these changes
@yuruguo yuruguo yuruguo approved these changes
@nicoloboschi nicoloboschi nicoloboschi approved these changes
@eolivelli eolivelli Awaiting requested review from eolivelli
@hangc0276 hangc0276 Awaiting requested review from hangc0276
@zymap zymap Awaiting requested review from zymap

Assignees

@thomasleplus thomasleplus

Labels

area/admin cherry-picked/branch-2.8 Archived: 2.8 is end of life doc-not-needed Your PR changes do not impact docs release/2.8.2

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@thomasleplus @yuruguo @315157973 @codelipenghui @nicoloboschi