pulsar icon indicating copy to clipboard operation
pulsar copied to clipboard

[fix][admin] Tenant AdminRoles can not contains whitespace in the beginning or end.

Open falser101 opened this issue 10 months ago • 6 comments

Fixes #21710

Verifying this change

  • [x] Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change added tests

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
  • [x] The admin CLI options
  • [ ] The metrics
  • [ ] Anything that affects deployment

Documentation

  • [ ] doc
  • [ ] doc-required
  • [x] doc-not-needed
  • [ ] doc-complete

falser101 avatar Apr 07 '24 03:04 falser101

please help review, thanks. @BewareMyPower @poorbarcode @codelipenghui @liangyepianzhou

falser101 avatar Apr 08 '24 01:04 falser101

please review, thanks. @BewareMyPower @poorbarcode @liangyepianzhou

falser101 avatar Apr 11 '24 05:04 falser101

Please fix the checkstyle

https://github.com/apache/pulsar/actions/runs/8624709432/job/23654223248?pr=2245 resoled @BewareMyPower

falser101 avatar Apr 18 '24 01:04 falser101

please help review @lhotari @liangyepianzhou

falser101 avatar May 18 '24 07:05 falser101

I think it would be ready to merge if tests passed since all comments should be addressed.

BewareMyPower avatar May 20 '24 02:05 BewareMyPower

/test

falser101 avatar May 21 '24 09:05 falser101

I think it would be ready to merge if tests passed since all comments should be addressed.

how to run test,it's waiting for status to be reported?

falser101 avatar May 24 '24 01:05 falser101

Since you're the first-time contributor, the workflow requires a committer to click the Approve and Run tests button to trigger the CI. I have clicked it just now. So let's wait

BewareMyPower avatar May 24 '24 03:05 BewareMyPower

There are some failed tests, please check them.

BewareMyPower avatar May 24 '24 05:05 BewareMyPower

There are some failed tests, please check them.

ok.

falser101 avatar May 25 '24 09:05 falser101

There are some failed tests, please check them.

resolved

falser101 avatar May 25 '24 15:05 falser101

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 73.21%. Comparing base (bbc6224) to head (1367c39). Report is 303 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #22450      +/-   ##
============================================
- Coverage     73.57%   73.21%   -0.36%     
- Complexity    32624    32902     +278     
============================================
  Files          1877     1890      +13     
  Lines        139502   141502    +2000     
  Branches      15299    15528     +229     
============================================
+ Hits         102638   103606     +968     
- Misses        28908    29903     +995     
- Partials       7956     7993      +37     
Flag Coverage Δ
inttests 27.41% <50.00%> (+2.83%) :arrow_up:
systests 24.62% <40.00%> (+0.30%) :arrow_up:
unittests 72.22% <100.00%> (-0.62%) :arrow_down:

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

Files Coverage Δ
...g/apache/pulsar/broker/admin/impl/TenantsBase.java 96.68% <100.00%> (+0.23%) :arrow_up:

... and 356 files with indirect coverage changes

codecov-commenter avatar May 27 '24 03:05 codecov-commenter

@BewareMyPower help merge this pull requests,thanks

falser101 avatar May 27 '24 06:05 falser101