Skip to content

[improve][cli] handle help flag properly - #18832

Merged
nicoloboschi merged 4 commits into
apache:masterfrom
tisonkun:handle-help-flag
Dec 12, 2022
Merged

[improve][cli] handle help flag properly#18832
nicoloboschi merged 4 commits into
apache:masterfrom
tisonkun:handle-help-flag

Conversation

@tisonkun

@tisonkun tisonkun commented Dec 9, 2022

Copy link
Copy Markdown
Member

Motivation

Currently

$ ./bin/pulsar-admin schemas -h


Expected a command, got -h

Invalid command, please use `pulsar-admin --help` to check out how to use
2022-12-09T16:18:27,751+0800 [main] WARN  org.apache.pulsar.common.util.ShutdownUtil - Triggering immediate shutdown of current process with status 1
java.lang.Exception: Stacktrace for immediate shutdown
	at org.apache.pulsar.common.util.ShutdownUtil.triggerImmediateForcefulShutdown(ShutdownUtil.java:52) ~[pulsar-common.jar:2.11.0-SNAPSHOT]
	at org.apache.pulsar.admin.cli.PulsarAdminTool.exit(PulsarAdminTool.java:309) ~[pulsar-client-tools.jar:2.11.0-SNAPSHOT]
	at org.apache.pulsar.admin.cli.PulsarAdminTool.main(PulsarAdminTool.java:300) ~[pulsar-client-tools.jar:2.11.0-SNAPSHOT]
$ ./bin/pulsar-admin schemas delete -h


null

Reason: java.util.concurrent.CompletionException: org.apache.pulsar.client.admin.internal.http.AsyncHttpConnector$RetryException: Could not complete the operation. Number of retries has been exhausted. Failed reason: Connection refused: localhost/127.0.0.1:8080
2022-12-09T16:18:32,984+0800 [main] WARN  org.apache.pulsar.common.util.ShutdownUtil - Triggering immediate shutdown of current process with status 1
java.lang.Exception: Stacktrace for immediate shutdown
	at org.apache.pulsar.common.util.ShutdownUtil.triggerImmediateForcefulShutdown(ShutdownUtil.java:52) ~[pulsar-common.jar:2.11.0-SNAPSHOT]
	at org.apache.pulsar.admin.cli.PulsarAdminTool.exit(PulsarAdminTool.java:309) ~[pulsar-client-tools.jar:2.11.0-SNAPSHOT]
	at org.apache.pulsar.admin.cli.PulsarAdminTool.main(PulsarAdminTool.java:300) ~[pulsar-client-tools.jar:2.11.0-SNAPSHOT]

With this patch

$ ./bin/pulsar-admin schemas -h       


Usage: pulsar-admin schemas [options] [command] [command options]
  Commands:
    get      Get the schema for a topic
      Usage: get [options] persistent://tenant/namespace/topic
        Options:
          -a, --all-version
            all version
            Default: false
          -v, --version
            version

    delete      Delete the latest schema for a topic
      Usage: delete [options] persistent://tenant/namespace/topic
        Options:
          -f, --force
            whether to delete schema completely. If true, delete all resources 
            (including metastore and ledger), otherwise only do a mark 
            deletion and not remove any resources indeed
            Default: false

    upload      Update the schema for a topic
      Usage: upload [options] persistent://tenant/namespace/topic
        Options:
        * -f, --filename
            filename

    extract      Provide the schema via a topic
      Usage: extract [options] persistent://tenant/namespace/topic
        Options:
          -a, --always-allow-null
            set schema whether always allow null or not
            Default: true
        * -c, --classname
            class name of pojo
          -n, --dry-run
            dost not apply to schema registry, just prints the post schema 
            payload 
            Default: false
        * -j, --jar
            jar filepath
        * -t, --type
            type avro or json
$ ./bin/pulsar-admin schemas delete -h


Usage: pulsar-admin schemas delete [options] 
      persistent://tenant/namespace/topic 
  Options:
    -f, --force
      whether to delete schema completely. If true, delete all resources 
      (including metastore and ledger), otherwise only do a mark deletion and 
      not remove any resources indeed
      Default: false

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

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

Signed-off-by: tison <wander4096@gmail.com>
@tisonkun tisonkun self-assigned this Dec 9, 2022
@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Dec 9, 2022
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
@codecov-commenter

codecov-commenter commented Dec 9, 2022

Copy link
Copy Markdown

Codecov Report

Merging #18832 (0ee1a52) into master (da87e40) will increase coverage by 1.25%.
The diff coverage is 42.85%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #18832      +/-   ##
============================================
+ Coverage     46.42%   47.68%   +1.25%     
- Complexity    10437    10625     +188     
============================================
  Files           703      703              
  Lines         68816    68838      +22     
  Branches       7377     7379       +2     
============================================
+ Hits          31950    32822     +872     
+ Misses        33252    32336     -916     
- Partials       3614     3680      +66     
Flag Coverage Δ
unittests 47.68% <42.85%> (+1.25%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ache/pulsar/broker/loadbalance/LinuxInfoUtils.java 26.74% <0.00%> (-1.17%) ⬇️
...ker/resourcegroup/ResourceQuotaCalculatorImpl.java 4.16% <0.00%> (ø)
...ker/loadbalance/impl/LinuxBrokerHostUsageImpl.java 79.68% <100.00%> (+0.32%) ⬆️
.../service/SystemTopicBasedTopicPoliciesService.java 72.56% <100.00%> (+8.14%) ⬆️
...ker/loadbalance/impl/LeastLongTermMessageRate.java 73.33% <0.00%> (-20.00%) ⬇️
...ersistentStickyKeyDispatcherMultipleConsumers.java 46.07% <0.00%> (-14.71%) ⬇️
...ervice/persistent/MessageRedeliveryController.java 56.09% <0.00%> (-9.76%) ⬇️
...ker/resourcegroup/ResourceGroupConfigListener.java 57.53% <0.00%> (-6.85%) ⬇️
...g/apache/pulsar/client/impl/ConnectionHandler.java 50.00% <0.00%> (-5.32%) ⬇️
...g/apache/pulsar/broker/lookup/TopicLookupBase.java 50.27% <0.00%> (-3.92%) ⬇️
... and 81 more

Signed-off-by: tison <wander4096@gmail.com>

@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.

LGTM

@nicoloboschi
nicoloboschi merged commit 25866a3 into apache:master Dec 12, 2022
@tisonkun
tisonkun deleted the handle-help-flag branch December 12, 2022 08:59
Demogorgon314 pushed a commit to Demogorgon314/pulsar that referenced this pull request Dec 26, 2022
Signed-off-by: tison <wander4096@gmail.com>
Demogorgon314 pushed a commit to Demogorgon314/pulsar that referenced this pull request Dec 29, 2022
Signed-off-by: tison <wander4096@gmail.com>
lifepuzzlefun pushed a commit to lifepuzzlefun/pulsar that referenced this pull request Jan 10, 2023
Signed-off-by: tison <wander4096@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/cli doc-not-needed Your PR changes do not impact docs ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants