You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/12/15 21:39:53 UTC

[GitHub] [pulsar] pgier opened a new pull request, #18946: [improve][cli] pulsar cli: use return code 1 for general error

pgier opened a new pull request, #18946:
URL: https://github.com/apache/pulsar/pull/18946

   Return code -1 is generally considered to be an invalid code. https://tldp.org/LDP/abs/html/exitcodes.html
   Replace return code -1 with 1 which is commonly used as a general error.
   
   Signed-off-by: Paul Gier <pa...@datastax.com>
   
   <!--
   ### Contribution Checklist
     
     - PR title format should be *[type][component] summary*. For details, see *[Guideline - Pulsar PR Naming Convention](https://docs.google.com/document/d/1d8Pw6ZbWk-_pCKdOmdvx9rnhPiyuxwq60_TrD68d7BA/edit#heading=h.trs9rsex3xom)*. 
   
     - Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
     
     - Each pull request should address only one issue, not mix up code from multiple issues.
     
     - Each commit in the pull request has a meaningful commit message
   
     - Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below.
   -->
   
   <!-- Either this PR fixes an issue, -->
   
   Fixes #18945
   
   ### Motivation
   
   There are a lot of cases in the pulsar cli tools where a `-1` is returned as the return code when there is an error executing the command.  This is generally considered an invalid return code and the code should be an integer between 0 and 255, with `1` commonly used as a general error code when specific error codes are not defined.  Because `-1` is invalid, running a pulsar command which fails will actually return a `255` because `255` is used to indicate that the return code is out of range.
   
   ### Modifications
   
   Updated the CLI commands to return `1` instead of `-1` when there is an error.
   
   ### Verifying this change
   
   - [X] 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:
   
   *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
   
   I'm not sure if there are other applications which depend on the current specific return codes, but it's doubtful that there are many cases.  Normally, scripts just check for a non-zero return code and don't look specifically for a 255.  If there are a significant number of users that depend directly on the 255 return code, then we might have to make this change more gradually.
   
   ### Documentation
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   - [ ] `doc` <!-- Your PR contains doc changes. Please attach the local preview screenshots (run `sh start.sh` at `pulsar/site2/website`) to your PR description, or else your PR might not get merged. -->
   - [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
   - [X] `doc-not-needed` <!-- Your PR changes do not impact docs -->
   - [ ] `doc-complete` <!-- Docs have been already added -->
   
   ### Matching PR in forked repository
   
   PR in forked repository: <!-- ENTER URL HERE -->
   
   <!--
   After opening this PR, the build in apache/pulsar will fail and instructions will
   be provided for opening a PR in the PR author's forked repository.
   
   apache/pulsar pull requests should be first tested in your own fork since the 
   apache/pulsar CI based on GitHub Actions has constrained resources and quota.
   GitHub Actions provides separate quota for pull requests that are executed in 
   a forked repository.
   
   The tests will be run in the forked repository until all PR review comments have
   been handled, the tests pass and the PR is approved by a reviewer.
   -->
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] tisonkun commented on pull request #18946: [improve][cli] pulsar cli: use return code 1 for general error

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #18946:
URL: https://github.com/apache/pulsar/pull/18946#issuecomment-1353849635

   /pulsarbot run-failure-checks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] pgier commented on pull request #18946: [improve][cli] pulsar cli: use return code 1 for general error

Posted by GitBox <gi...@apache.org>.
pgier commented on PR #18946:
URL: https://github.com/apache/pulsar/pull/18946#issuecomment-1355740354

   I think the remaining CI failure is a false positive.  All the tests passed in my forked repo.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] nicoloboschi merged pull request #18946: [improve][cli] pulsar cli: use return code 1 for general error

Posted by GitBox <gi...@apache.org>.
nicoloboschi merged PR #18946:
URL: https://github.com/apache/pulsar/pull/18946


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] codecov-commenter commented on pull request #18946: [improve][cli] pulsar cli: use return code 1 for general error

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #18946:
URL: https://github.com/apache/pulsar/pull/18946#issuecomment-1353907400

   # [Codecov](https://codecov.io/gh/apache/pulsar/pull/18946?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#18946](https://codecov.io/gh/apache/pulsar/pull/18946?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (faf59dc) into [master](https://codecov.io/gh/apache/pulsar/commit/3011946a5c3b64ed7c08b6bfb1f6492f8aaaca9c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3011946) will **decrease** coverage by `12.29%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pulsar/pull/18946/graphs/tree.svg?width=650&height=150&src=pr&token=acYqCpsK9J&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/pulsar/pull/18946?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             master   #18946       +/-   ##
   =============================================
   - Coverage     46.75%   34.45%   -12.30%     
   + Complexity    10514     6594     -3920     
   =============================================
     Files           703      623       -80     
     Lines         68858    59106     -9752     
     Branches       7383     6147     -1236     
   =============================================
   - Hits          32193    20365    -11828     
   - Misses        33065    36016     +2951     
   + Partials       3600     2725      -875     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | unittests | `34.45% <0.00%> (-12.30%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/pulsar/pull/18946?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...pache/pulsar/proxy/server/ProxyServiceStarter.java](https://codecov.io/gh/apache/pulsar/pull/18946/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLXByb3h5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9wdWxzYXIvcHJveHkvc2VydmVyL1Byb3h5U2VydmljZVN0YXJ0ZXIuamF2YQ==) | `60.66% <0.00%> (ø)` | |
   | [...ava/org/apache/pulsar/broker/admin/v1/Brokers.java](https://codecov.io/gh/apache/pulsar/pull/18946/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi92MS9Ccm9rZXJzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...va/org/apache/pulsar/broker/admin/v1/Clusters.java](https://codecov.io/gh/apache/pulsar/pull/18946/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi92MS9DbHVzdGVycy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../org/apache/pulsar/broker/admin/v1/Properties.java](https://codecov.io/gh/apache/pulsar/pull/18946/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi92MS9Qcm9wZXJ0aWVzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../apache/pulsar/broker/admin/v2/ResourceGroups.java](https://codecov.io/gh/apache/pulsar/pull/18946/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi92Mi9SZXNvdXJjZUdyb3Vwcy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ar/common/naming/PartitionedManagedLedgerInfo.java](https://codecov.io/gh/apache/pulsar/pull/18946/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NvbW1vbi9uYW1pbmcvUGFydGl0aW9uZWRNYW5hZ2VkTGVkZ2VySW5mby5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...e/pulsar/broker/admin/impl/ResourceQuotasBase.java](https://codecov.io/gh/apache/pulsar/pull/18946/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi9pbXBsL1Jlc291cmNlUXVvdGFzQmFzZS5qYXZh) | `0.00% <0.00%> (-96.43%)` | :arrow_down: |
   | [...he/pulsar/broker/service/AnalyzeBacklogResult.java](https://codecov.io/gh/apache/pulsar/pull/18946/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL0FuYWx5emVCYWNrbG9nUmVzdWx0LmphdmE=) | `0.00% <0.00%> (-92.31%)` | :arrow_down: |
   | [.../apache/pulsar/broker/admin/v1/ResourceQuotas.java](https://codecov.io/gh/apache/pulsar/pull/18946/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi92MS9SZXNvdXJjZVF1b3Rhcy5qYXZh) | `0.00% <0.00%> (-91.43%)` | :arrow_down: |
   | [...e/pulsar/broker/stats/AllocatorStatsGenerator.java](https://codecov.io/gh/apache/pulsar/pull/18946/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zdGF0cy9BbGxvY2F0b3JTdGF0c0dlbmVyYXRvci5qYXZh) | `0.00% <0.00%> (-91.38%)` | :arrow_down: |
   | ... and [197 more](https://codecov.io/gh/apache/pulsar/pull/18946/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org