Skip to content

[cli] Fix output format of string by pulsar-admin command - #11878

Merged
nkurihar merged 3 commits into
apache:masterfrom
massakam:fix-pulsar-admin-output-format
Oct 25, 2021
Merged

[cli] Fix output format of string by pulsar-admin command#11878
nkurihar merged 3 commits into
apache:masterfrom
massakam:fix-pulsar-admin-output-format

Conversation

@massakam

@massakam massakam commented Sep 1, 2021

Copy link
Copy Markdown
Contributor

Motivation

In Pulsar version 2.4.2 and earlier, the pulsar-admin command output a list of strings in the following format:

$ ./bin/pulsar-admin tenants list

public
pulsar
sample

On the other hand, in 2.5.0 and later versions, each string is enclosed in double quotes:

$ ./bin/pulsar-admin tenants list

"public"
"pulsar"
"sample"

This is due to #5313. With this change, strings are now displayed after being encoded to JSON. I think these double quotes are superfluous.

Modifications

If the type of the variable to be output is String, it will be output without JSON encoding.

Verifying this change

  • Make sure that the change passes the CI checks.

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

If yes was chosen, please highlight the changes

  • 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)

@massakam massakam added area/cli doc-not-needed Your PR changes do not impact docs labels Sep 1, 2021
@massakam massakam added this to the 2.9.0 milestone Sep 1, 2021
@massakam massakam self-assigned this Sep 1, 2021
@massakam
massakam force-pushed the fix-pulsar-admin-output-format branch 4 times, most recently from 3039123 to 824f109 Compare September 2, 2021 08:11
@massakam massakam changed the title [cli] Fix output format of list of strings by pulsar-admin command [cli] Fix output format of string by pulsar-admin command Sep 2, 2021
@massakam massakam added doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. and removed doc-not-needed Your PR changes do not impact docs labels Sep 2, 2021
@Anonymitaet

Copy link
Copy Markdown
Member

Hi @tuteng Could you please help review this PR from the technical perspective? Thanks

@massakam
massakam requested a review from tuteng September 29, 2021 04:19
@eolivelli eolivelli modified the milestones: 2.9.0, 2.10.0 Oct 6, 2021
@massakam
massakam force-pushed the fix-pulsar-admin-output-format branch from bbe19b4 to e3c481c Compare October 13, 2021 02:50
@massakam
massakam force-pushed the fix-pulsar-admin-output-format branch from e3c481c to 7dad9b0 Compare October 14, 2021 02:10
@nkurihar

Copy link
Copy Markdown
Contributor

@rdhabalia @sijie @merlimat
Any thoughts for this PR?

It looks good to 2.4.2 or earlier users (including me).
However it might be "breaking" for 2.5.0 or later users.

IMO the impact of this change is within only displayed messages and not large, so I approved.

@nkurihar
nkurihar merged commit 7037c5e into apache:master Oct 25, 2021
@massakam
massakam deleted the fix-pulsar-admin-output-format branch October 25, 2021 01:55
zeo1995 pushed a commit to zeo1995/pulsar that referenced this pull request Oct 25, 2021
* up/master: (46 commits)
  [website][upgrade]feat: docs migration - version-2.7.2 Pulsar Schema (apache#12393)
  [docs] io-develop, fix broken link (apache#12414)
  docs(function): fix incorrect classname in python runtime sample (apache#12476)
  Remove redundant null check for getInternalListener (apache#12474)
  Fix the retry topic's `REAL_TOPIC` & `ORIGIN_MESSAGE_ID` property should not be modified once it has been written. (apache#12451)
  [cli] Fix output format of string by pulsar-admin command (apache#11878)
  fix the race of delete subscription and delete topic (apache#12240)
  fix influxdb yaml doc (apache#12460)
  [Modernizer] Add Maven Modernizer plugin in pulsar-proxy module (apache#12326)
  fix DefaultCryptoKeyReaderTest can not run on windows (apache#12475)
  apache#12429 only fixed the compactor skips data issue, but the normal reader/consumer (apache#12464)
  broker resource group test optimize fail msg (apache#12438)
  Stop OffsetStore when stopping the connector (apache#12457)
  fix a typo in UnAckedMessageTracker (apache#12467)
  docs(function): fix typo in pip install (apache#12468)
  Optimize the code: remove extra spaces (apache#12470)
  optimize SecurityUtility code flow (apache#12431)
  Update lombok to 1.18.22 (apache#12466)
  Update team.js to add David K. as a committer (apache#12440)
  Fix java demo error in reset cursor admin (apache#12454)
  ...

# Conflicts:
#	site2/website-next/versioned_docs/version-2.7.2/schema-evolution-compatibility.md
#	site2/website-next/versioned_docs/version-2.7.2/schema-get-started.md
#	site2/website-next/versioned_docs/version-2.7.2/schema-manage.md
#	site2/website-next/versioned_docs/version-2.7.2/schema-understand.md
#	site2/website-next/versioned_sidebars/version-2.7.2-sidebars.json
eolivelli pushed a commit to eolivelli/pulsar that referenced this pull request Nov 29, 2021
* Fix output format of list of strings by pulsar-admin command

* Fix test code

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

Labels

area/cli doc Your PR contains doc changes, no matter whether the changes are in markdown or code files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants