Skip to content

[pulsar-admin] Perfect judgment conditions of pulsar-admin #12315

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
Merged
codelipenghui merged 1 commit into from
Nov 8, 2021
Merged

[pulsar-admin] Perfect judgment conditions of pulsar-admin #12315

codelipenghui merged 1 commit into from
Nov 8, 2021

Conversation

@yuruguo
Copy link
Contributor

@yuruguo yuruguo commented Oct 10, 2021 •
edited
Loading

Motivation

Perfect judgment conditions of bin/pulsar-admin, included parameter CONF_FILE_PATH and field serviceUrl.

Details are as follows:
First of all, the configuration file CONF_FILE_PATH is a required option rather than an optional option (from the source code, the configuration file is regarded as an optional option) , because some very important default values (such as webServiceUrl=http://localhost:8080/, brokerServiceUrl=pulsar://localhost:6650/) cannot be obtained without a configuration file, and will mistake args[0] as the configuration file, as follows:

./bin/pulsar-admin topics list-partitioned-topics test/app1
Exception in thread "main" java.io.FileNotFoundException: topics (No such file or directory)
	at java.io.FileInputStream.open0(Native Method)
	at java.io.FileInputStream.open(FileInputStream.java:195)
	at java.io.FileInputStream.<init>(FileInputStream.java:138)
	at java.io.FileInputStream.<init>(FileInputStream.java:93)
	at org.apache.pulsar.admin.cli.PulsarAdminTool.main(PulsarAdminTool.java:296)

so we first need to judge whether the configuration file CONF_FILE_PATH exists, and give the correct command line format (Usage: pulsar-admin CONF_FILE_PATH [options] [command] [command options]) if it is illegal.

Secondly, serviceUrl is a very important field, because pulsarAdmin can connect to the pulsar server through this value. If the configuration file does not have releated key=value(such as webServiceUrl=, brokerServiceUrl=) and the user does not specify option --admin-url, then NPE will appear, as follows:

./bin/pulsar-admin topics list-partitioned-topics test/app1
java.lang.NullPointerException
	at org.apache.pulsar.client.admin.internal.PulsarAdminImpl.<init>(PulsarAdminImpl.java:189)
	at org.apache.pulsar.client.admin.internal.PulsarAdminBuilderImpl.build(PulsarAdminBuilderImpl.java:47)
	at org.apache.pulsar.admin.cli.PulsarAdminTool.lambda$main$2(PulsarAdminTool.java:320)
	at org.apache.pulsar.admin.cli.PulsarAdminTool$PulsarAdminSupplier.get(PulsarAdminTool.java:174)
	at org.apache.pulsar.admin.cli.PulsarAdminTool$PulsarAdminSupplier.get(PulsarAdminTool.java:161)
	at org.apache.pulsar.admin.cli.CmdBase.getAdmin(CmdBase.java:111)
	at org.apache.pulsar.admin.cli.CmdTopics.getTopics(CmdTopics.java:2360)
	at org.apache.pulsar.admin.cli.CmdTopics.access$11000(CmdTopics.java:73)
	at org.apache.pulsar.admin.cli.CmdTopics$PartitionedTopicListCmd.run(CmdTopics.java:275)
	at org.apache.pulsar.admin.cli.CmdBase.run(CmdBase.java:86)
	at org.apache.pulsar.admin.cli.PulsarAdminTool.run(PulsarAdminTool.java:282)
	at org.apache.pulsar.admin.cli.PulsarAdminTool.main(PulsarAdminTool.java:330)
class java.lang.NullPointerException: null

so we need to judge whether field serviceUrl exists.

Documentation

  • no-need-doc

@yuruguo yuruguo changed the title [client-tools] Optimize execution timing of jcommander.parse [pulsar-admin] Perfect judgment conditions of pulsar-admin Oct 10, 2021
@gaoran10
Copy link
Contributor

gaoran10 commented Oct 10, 2021

Could you add more context about this change? It seems that your pulsar-admin command didn't find the config file.

@yuruguo
Copy link
Contributor Author

yuruguo commented Oct 11, 2021 •
edited
Loading

Could you add more context about this change? It seems that your pulsar-admin command didn't find the config file.

A detailed description has been added.
Please take a look for this PR, thx!

@Anonymitaet Anonymitaet added the doc-not-needed Your PR changes do not impact docs label Oct 13, 2021
@gaoran10
Copy link
Contributor

gaoran10 commented Oct 14, 2021

Did you move the configuration file? Then this exception occurs.

./bin/pulsar-admin topics list-partitioned-topics test/app1
Exception in thread "main" java.io.FileNotFoundException: topics (No such file or directory)
	at java.io.FileInputStream.open0(Native Method)
	at java.io.FileInputStream.open(FileInputStream.java:195)
	at java.io.FileInputStream.<init>(FileInputStream.java:138)
	at java.io.FileInputStream.<init>(FileInputStream.java:93)
	at org.apache.pulsar.admin.cli.PulsarAdminTool.main(PulsarAdminTool.java:296)

@yuruguo
Copy link
Contributor Author

yuruguo commented Oct 14, 2021

Did you move the configuration file? Then this exception occurs.

./bin/pulsar-admin topics list-partitioned-topics test/app1
Exception in thread "main" java.io.FileNotFoundException: topics (No such file or directory)
	at java.io.FileInputStream.open0(Native Method)
	at java.io.FileInputStream.open(FileInputStream.java:195)
	at java.io.FileInputStream.<init>(FileInputStream.java:138)
	at java.io.FileInputStream.<init>(FileInputStream.java:93)
	at org.apache.pulsar.admin.cli.PulsarAdminTool.main(PulsarAdminTool.java:296)

you are right

@yuruguo yuruguo requested a review from hangc0276 November 2, 2021 04:52
@codelipenghui codelipenghui added this to the 2.10.0 milestone Nov 4, 2021
@codelipenghui codelipenghui merged commit f976500 into apache:master Nov 8, 2021
zeo1995 pushed a commit to zeo1995/pulsar that referenced this pull request Nov 9, 2021
Merge branch 'up/master' into website/branch-2.7.2-chapter-6
aee5758 
* up/master:
  [Doc] Add explanations for setting geo-replication at topic level (apache#12633)
  commit chapter Tiered Storage (apache#12592)
  [pulsar-admin] Add remove-subscription-types-enabled command for namespace (apache#12392)
  Enable CLI to publish non-batched messages (apache#12641)
  [Doc] Add doc for tokenSettingPrefix (apache#12662)
  [pulsar-admin] Add corresponding get command for namespace (apache#12322)
  [pulsar-admin] Perfect judgment conditions of pulsar-admin (apache#12315)
  [broker] Avoid unnecessary recalculation of maxSubscriptionsPerTopic in AbstractTopic (apache#12658)
  [Transaction]Stop TB recovering with exception (apache#12636)
  [website][upgrade]feat: docs migration - 2.7.1 / client (apache#12612)
  [website][upgrade]feat: docs migration - 2.7.1 / performance (apache#12611)
  [website][upgrade]feat: docs migration - 2.7.1 / security (apache#12610)
  [Modernizer] Apply Modernizer plugin for pulsar broker common module and fix violation. (apache#12657)
  [Authorization] Support GET_METADATA topic op after enable auth (apache#12656)
  Fix StringIndexOutOfBoundsException in org.apache.pulsar.broker.resources.NamespaceResources#pathIsFromNamespace (apache#12659)
zeo1995 pushed a commit to zeo1995/pulsar that referenced this pull request Nov 9, 2021
Merge branch 'up/master' into website/branch-2.7.2-chapter-5
ee04033 
* up/master:
  [Doc] Add explanations for setting geo-replication at topic level (apache#12633)
  commit chapter Tiered Storage (apache#12592)
  [pulsar-admin] Add remove-subscription-types-enabled command for namespace (apache#12392)
  Enable CLI to publish non-batched messages (apache#12641)
  [Doc] Add doc for tokenSettingPrefix (apache#12662)
  [pulsar-admin] Add corresponding get command for namespace (apache#12322)
  [pulsar-admin] Perfect judgment conditions of pulsar-admin (apache#12315)
  [broker] Avoid unnecessary recalculation of maxSubscriptionsPerTopic in AbstractTopic (apache#12658)
  [Transaction]Stop TB recovering with exception (apache#12636)
  [website][upgrade]feat: docs migration - 2.7.1 / client (apache#12612)
  [website][upgrade]feat: docs migration - 2.7.1 / performance (apache#12611)
  [website][upgrade]feat: docs migration - 2.7.1 / security (apache#12610)
  [Modernizer] Apply Modernizer plugin for pulsar broker common module and fix violation. (apache#12657)
  [Authorization] Support GET_METADATA topic op after enable auth (apache#12656)
  Fix StringIndexOutOfBoundsException in org.apache.pulsar.broker.resources.NamespaceResources#pathIsFromNamespace (apache#12659)

# Conflicts:
#	site2/website-next/versioned_sidebars/version-2.7.2-sidebars.json
codelipenghui pushed a commit that referenced this pull request Nov 18, 2021
@yuruguo @codelipenghui
[pulsar-admin] Perfect judgment conditions of pulsar-admin (#12315)
a94dbe6 
### Motivation
Perfect judgment conditions of `bin/pulsar-admin`, included parameter `CONF_FILE_PATH` and field `serviceUrl`.

**Details are as follows:**
**First of all**, the configuration file `CONF_FILE_PATH` is a required option rather than an optional option (from the [source code](https://github.com/apache/pulsar/blob/master/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/PulsarAdminTool.java#L288-L299), the configuration file is regarded as an optional option) , because some very important [default values](https://github.com/apache/pulsar/blob/master/conf/client.conf#L22-L30) (such as `webServiceUrl=http://localhost:8080/`, `brokerServiceUrl=pulsar://localhost:6650/`) cannot be obtained without a configuration file, and will mistake args[0] as the configuration file, as follows:
```
./bin/pulsar-admin topics list-partitioned-topics test/app1
Exception in thread "main" java.io.FileNotFoundException: topics (No such file or directory)
	at java.io.FileInputStream.open0(Native Method)
	at java.io.FileInputStream.open(FileInputStream.java:195)
	at java.io.FileInputStream.<init>(FileInputStream.java:138)
	at java.io.FileInputStream.<init>(FileInputStream.java:93)
	at org.apache.pulsar.admin.cli.PulsarAdminTool.main(PulsarAdminTool.java:296)
```
so we first need to judge whether the configuration file `CONF_FILE_PATH` exists, and give the correct command line format (`Usage: pulsar-admin CONF_FILE_PATH [options] [command] [command options]`) if it is illegal.

**Secondly**, `serviceUrl` is a very important field, because pulsarAdmin can connect to the pulsar server through this value. If the configuration file does not have releated `key`=`value`(such as `webServiceUrl`=, `brokerServiceUrl`=) and the user does not specify option `--admin-url`, then NPE will appear, as follows:
```
./bin/pulsar-admin topics list-partitioned-topics test/app1
java.lang.NullPointerException
	at org.apache.pulsar.client.admin.internal.PulsarAdminImpl.<init>(PulsarAdminImpl.java:189)
	at org.apache.pulsar.client.admin.internal.PulsarAdminBuilderImpl.build(PulsarAdminBuilderImpl.java:47)
	at org.apache.pulsar.admin.cli.PulsarAdminTool.lambda$main$2(PulsarAdminTool.java:320)
	at org.apache.pulsar.admin.cli.PulsarAdminTool$PulsarAdminSupplier.get(PulsarAdminTool.java:174)
	at org.apache.pulsar.admin.cli.PulsarAdminTool$PulsarAdminSupplier.get(PulsarAdminTool.java:161)
	at org.apache.pulsar.admin.cli.CmdBase.getAdmin(CmdBase.java:111)
	at org.apache.pulsar.admin.cli.CmdTopics.getTopics(CmdTopics.java:2360)
	at org.apache.pulsar.admin.cli.CmdTopics.access$11000(CmdTopics.java:73)
	at org.apache.pulsar.admin.cli.CmdTopics$PartitionedTopicListCmd.run(CmdTopics.java:275)
	at org.apache.pulsar.admin.cli.CmdBase.run(CmdBase.java:86)
	at org.apache.pulsar.admin.cli.PulsarAdminTool.run(PulsarAdminTool.java:282)
	at org.apache.pulsar.admin.cli.PulsarAdminTool.main(PulsarAdminTool.java:330)
class java.lang.NullPointerException: null
```
so we need to judge whether field `serviceUrl` exists.

(cherry picked from commit f976500)
@codelipenghui codelipenghui added cherry-picked/branch-2.8 Archived: 2.8 is end of life release/2.8.2 and removed release/2.8.3 labels Nov 18, 2021
eolivelli pushed a commit to eolivelli/pulsar that referenced this pull request Nov 29, 2021
@yuruguo @eolivelli
[pulsar-admin] Perfect judgment conditions of pulsar-admin (apache#12315
c4952dd 
)

### Motivation
Perfect judgment conditions of `bin/pulsar-admin`, included parameter `CONF_FILE_PATH` and field `serviceUrl`.

**Details are as follows:**
**First of all**, the configuration file `CONF_FILE_PATH` is a required option rather than an optional option (from the [source code](https://github.com/apache/pulsar/blob/master/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/PulsarAdminTool.java#L288-L299), the configuration file is regarded as an optional option) , because some very important [default values](https://github.com/apache/pulsar/blob/master/conf/client.conf#L22-L30) (such as `webServiceUrl=http://localhost:8080/`, `brokerServiceUrl=pulsar://localhost:6650/`) cannot be obtained without a configuration file, and will mistake args[0] as the configuration file, as follows:
```
./bin/pulsar-admin topics list-partitioned-topics test/app1
Exception in thread "main" java.io.FileNotFoundException: topics (No such file or directory)
	at java.io.FileInputStream.open0(Native Method)
	at java.io.FileInputStream.open(FileInputStream.java:195)
	at java.io.FileInputStream.<init>(FileInputStream.java:138)
	at java.io.FileInputStream.<init>(FileInputStream.java:93)
	at org.apache.pulsar.admin.cli.PulsarAdminTool.main(PulsarAdminTool.java:296)
```
so we first need to judge whether the configuration file `CONF_FILE_PATH` exists, and give the correct command line format (`Usage: pulsar-admin CONF_FILE_PATH [options] [command] [command options]`) if it is illegal.

**Secondly**, `serviceUrl` is a very important field, because pulsarAdmin can connect to the pulsar server through this value. If the configuration file does not have releated `key`=`value`(such as `webServiceUrl`=, `brokerServiceUrl`=) and the user does not specify option `--admin-url`, then NPE will appear, as follows:
```
./bin/pulsar-admin topics list-partitioned-topics test/app1
java.lang.NullPointerException
	at org.apache.pulsar.client.admin.internal.PulsarAdminImpl.<init>(PulsarAdminImpl.java:189)
	at org.apache.pulsar.client.admin.internal.PulsarAdminBuilderImpl.build(PulsarAdminBuilderImpl.java:47)
	at org.apache.pulsar.admin.cli.PulsarAdminTool.lambda$main$2(PulsarAdminTool.java:320)
	at org.apache.pulsar.admin.cli.PulsarAdminTool$PulsarAdminSupplier.get(PulsarAdminTool.java:174)
	at org.apache.pulsar.admin.cli.PulsarAdminTool$PulsarAdminSupplier.get(PulsarAdminTool.java:161)
	at org.apache.pulsar.admin.cli.CmdBase.getAdmin(CmdBase.java:111)
	at org.apache.pulsar.admin.cli.CmdTopics.getTopics(CmdTopics.java:2360)
	at org.apache.pulsar.admin.cli.CmdTopics.access$11000(CmdTopics.java:73)
	at org.apache.pulsar.admin.cli.CmdTopics$PartitionedTopicListCmd.run(CmdTopics.java:275)
	at org.apache.pulsar.admin.cli.CmdBase.run(CmdBase.java:86)
	at org.apache.pulsar.admin.cli.PulsarAdminTool.run(PulsarAdminTool.java:282)
	at org.apache.pulsar.admin.cli.PulsarAdminTool.main(PulsarAdminTool.java:330)
class java.lang.NullPointerException: null
```
so we need to judge whether field `serviceUrl` exists.
codelipenghui pushed a commit that referenced this pull request Dec 20, 2021
@yuruguo @codelipenghui
[pulsar-admin] Perfect judgment conditions of pulsar-admin (#12315)
c4e491b 
### Motivation
Perfect judgment conditions of `bin/pulsar-admin`, included parameter `CONF_FILE_PATH` and field `serviceUrl`.

**Details are as follows:**
**First of all**, the configuration file `CONF_FILE_PATH` is a required option rather than an optional option (from the [source code](https://github.com/apache/pulsar/blob/master/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/PulsarAdminTool.java#L288-L299), the configuration file is regarded as an optional option) , because some very important [default values](https://github.com/apache/pulsar/blob/master/conf/client.conf#L22-L30) (such as `webServiceUrl=http://localhost:8080/`, `brokerServiceUrl=pulsar://localhost:6650/`) cannot be obtained without a configuration file, and will mistake args[0] as the configuration file, as follows:
```
./bin/pulsar-admin topics list-partitioned-topics test/app1
Exception in thread "main" java.io.FileNotFoundException: topics (No such file or directory)
	at java.io.FileInputStream.open0(Native Method)
	at java.io.FileInputStream.open(FileInputStream.java:195)
	at java.io.FileInputStream.<init>(FileInputStream.java:138)
	at java.io.FileInputStream.<init>(FileInputStream.java:93)
	at org.apache.pulsar.admin.cli.PulsarAdminTool.main(PulsarAdminTool.java:296)
```
so we first need to judge whether the configuration file `CONF_FILE_PATH` exists, and give the correct command line format (`Usage: pulsar-admin CONF_FILE_PATH [options] [command] [command options]`) if it is illegal.

**Secondly**, `serviceUrl` is a very important field, because pulsarAdmin can connect to the pulsar server through this value. If the configuration file does not have releated `key`=`value`(such as `webServiceUrl`=, `brokerServiceUrl`=) and the user does not specify option `--admin-url`, then NPE will appear, as follows:
```
./bin/pulsar-admin topics list-partitioned-topics test/app1
java.lang.NullPointerException
	at org.apache.pulsar.client.admin.internal.PulsarAdminImpl.<init>(PulsarAdminImpl.java:189)
	at org.apache.pulsar.client.admin.internal.PulsarAdminBuilderImpl.build(PulsarAdminBuilderImpl.java:47)
	at org.apache.pulsar.admin.cli.PulsarAdminTool.lambda$main$2(PulsarAdminTool.java:320)
	at org.apache.pulsar.admin.cli.PulsarAdminTool$PulsarAdminSupplier.get(PulsarAdminTool.java:174)
	at org.apache.pulsar.admin.cli.PulsarAdminTool$PulsarAdminSupplier.get(PulsarAdminTool.java:161)
	at org.apache.pulsar.admin.cli.CmdBase.getAdmin(CmdBase.java:111)
	at org.apache.pulsar.admin.cli.CmdTopics.getTopics(CmdTopics.java:2360)
	at org.apache.pulsar.admin.cli.CmdTopics.access$11000(CmdTopics.java:73)
	at org.apache.pulsar.admin.cli.CmdTopics$PartitionedTopicListCmd.run(CmdTopics.java:275)
	at org.apache.pulsar.admin.cli.CmdBase.run(CmdBase.java:86)
	at org.apache.pulsar.admin.cli.PulsarAdminTool.run(PulsarAdminTool.java:282)
	at org.apache.pulsar.admin.cli.PulsarAdminTool.main(PulsarAdminTool.java:330)
class java.lang.NullPointerException: null
```
so we need to judge whether field `serviceUrl` exists.

(cherry picked from commit f976500)
@codelipenghui codelipenghui added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hangc0276 hangc0276 hangc0276 approved these changes

@codelipenghui codelipenghui codelipenghui approved these changes

@congbobo184 congbobo184 Awaiting requested review from congbobo184

@zymap zymap Awaiting requested review from zymap

Assignees

@yuruguo yuruguo

Labels

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

Projects

None yet

Milestone

2.10.0

Development

Successfully merging this pull request may close these issues.

6 participants

@yuruguo @gaoran10 @hangc0276 @codelipenghui @eolivelli @Anonymitaet