[refactor][cli][PIP-280] Create new `pulsar-cli-utils` module by JooHyukKim · Pull Request #20782 · apache/pulsar

JooHyukKim · 2023-07-11T15:23:42Z

PIP: #20691

Motivation

As PIP-280 #20691 may span cross module, we need a module to provide CLI related (though mostly JCommander atm) isolation. Note that pulsar-common was not considered because it's common (too broad).

Modifications

Add new module pulsar-cli-utils
Implement Converters for meaurement unit (time,byte) conversion
Implement Validators for

Note

This PR blocks [refactor][cli][PIP-280] Refactor pulsar-client-tools module #20764

Verifying this change

Make sure that the change passes the CI checks.

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

If the box was checked, please highlight the changes

Documentation

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

Matching PR in forked repository

PR in forked repository: https://github.com/JooHyukKim/pulsar/pull/19

codecov-commenter · 2023-07-12T06:36:05Z

Codecov Report

❌ Patch coverage is 92.70833% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.14%. Comparing base (d6734b7) to head (b4b0ba7).
⚠️ Report is 1724 commits behind head on master.

Files with missing lines	Patch %	Lines
...lsar/cli/converters/TimeUnitToMillisConverter.java	71.42%	2 Missing ⚠️
...sar/cli/converters/TimeUnitToSecondsConverter.java	71.42%	2 Missing ⚠️
...ava/org/apache/pulsar/cli/ValueValidationUtil.java	93.75%	1 Missing ⚠️
...org/apache/pulsar/cli/converters/ByteUnitUtil.java	94.73%	1 Missing ⚠️
...apache/pulsar/cli/converters/RelativeTimeUtil.java	95.23%	1 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             master   #20782       +/-   ##
=============================================
+ Coverage     36.84%   73.14%   +36.30%     
- Complexity    12195    32204    +20009     
=============================================
  Files          1698     1887      +189     
  Lines        129852   139539     +9687     
  Branches      14161    15337     +1176     
=============================================
+ Hits          47843   102067    +54224     
+ Misses        75680    29406    -46274     
- Partials       6329     8066     +1737

Flag	Coverage Δ
inttests	`24.10% <ø> (-0.03%)`	⬇️
systests	`25.11% <ø> (+0.01%)`	⬆️
unittests	`72.43% <92.70%> (+40.37%)`	⬆️

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

Files with missing lines	Coverage Δ
...ulsar/cli/converters/ByteUnitIntegerConverter.java	`100.00% <100.00%> (ø)`
...pulsar/cli/converters/ByteUnitToLongConverter.java	`100.00% <100.00%> (ø)`
...r/cli/validators/IntegerMaxValueLongValidator.java	`100.00% <100.00%> (ø)`
...pulsar/cli/validators/MinNegativeOneValidator.java	`100.00% <100.00%> (ø)`
...lsar/cli/validators/NonNegativeValueValidator.java	`100.00% <100.00%> (ø)`
.../cli/validators/PositiveIntegerValueValidator.java	`100.00% <100.00%> (ø)`
...sar/cli/validators/PositiveLongValueValidator.java	`100.00% <100.00%> (ø)`
...ava/org/apache/pulsar/cli/ValueValidationUtil.java	`93.75% <93.75%> (ø)`
...org/apache/pulsar/cli/converters/ByteUnitUtil.java	`94.73% <94.73%> (ø)`
...apache/pulsar/cli/converters/RelativeTimeUtil.java	`95.23% <95.23%> (ø)`
... and 2 more

... and 1438 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

JooHyukKim · 2023-07-12T10:50:06Z

/pulsarbot rerun-failure-checks

JooHyukKim · 2023-07-12T10:52:15Z

Re-running the following failed checks

OWASP dependency check : seems like it has to with rate limiting.
Pulsar IO - Oracle Browser just freezes, maybe too much log? (I will check other completed Actions to verify)

EDIT : Made a PR in to cover OWASP check #20792

…odule

JooHyukKim · 2023-07-13T02:48:15Z

Need #20792 to resolve the OWASP blocker

…odule

JooHyukKim · 2023-07-16T11:13:25Z

May I ask for a review when you have time 😆, @tisonkun @BewareMyPower @mattisonchao ? thanks 🙏🏼

JooHyukKim · 2023-07-19T01:15:19Z

/pulsarbot rerun-failure-checks

…odule

JooHyukKim · 2023-07-19T02:17:30Z

#20792 should fixs the OWASP check currently failing, but the suppression turns out to be more difficult than it seems. Any help will be welcome

tisonkun · 2023-07-20T10:25:49Z

@JooHyukKim you can now rebase this branch on the fixed master :D

…odule

gaoran10

Great work! LGTM

Co-authored-by: ran <gaoran_10@126.com>

BewareMyPower

Overall LGTM. Just a few comments about the naming.

You use Util for RelativeTimeUtil and ByteUnitUtil and Utils for ValueValidationUtils. Should we use a consistent naming style?

…odule

JooHyukKim · 2023-08-16T14:21:19Z

/pulsarbot rerun-failure-checks

JooHyukKim · 2023-08-17T12:41:26Z

/pulsarbot rerun-failure-checks

tisonkun

LGTM. Thanks for your contribution!

tisonkun · 2023-08-23T11:24:25Z

@JooHyukKim Maybe you can open a tracking issue to list out the subtasks and finish them one by one. In this way, we can share the same page what is remaining and maybe offer some helps.

JooHyukKim · 2023-08-23T13:43:38Z

@tisonkun Oh yes sounds like a good idea. Will do 👍🏻

…20782) Co-authored-by: ran <gaoran_10@126.com> (cherry picked from commit 4f9b199)

JooHyukKim added 9 commits July 11, 2023 21:53

Add new module

5f4b14f

Fix maven

4945b0e

Fix compile error wrt validation utils

ceb3fc4

Add apache commons dep

45b2120

Add test for ValueValidationUtilsTest

963a7e3

Refactor test methods

cbdda80

Fix tests and compile errors

346a361

Fix checkstyle

25a93ab

Fix build failure wrt findBugsFIlter

b4e2971

github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Jul 11, 2023

tisonkun added the ready-to-test label Jul 12, 2023

Merge remote-tracking branch 'upstream/master' into pip280-separate-m…

91e5caf

…odule

Merge remote-tracking branch 'upstream/master' into pip280-separate-m…

848fe00

…odule

JooHyukKim mentioned this pull request Jul 16, 2023

[feat][cli] Add command line option for configuring the memory limit #20663

Merged

15 tasks

tisonkun self-requested a review July 17, 2023 06:13

Merge remote-tracking branch 'upstream/master' into pip280-separate-m…

e28e104

…odule

Merge remote-tracking branch 'upstream/master' into pip280-separate-m…

59a6bed

…odule

BewareMyPower assigned JooHyukKim Jul 21, 2023

BewareMyPower requested review from BewareMyPower, gaoran10 and mattisonchao July 21, 2023 05:26

BewareMyPower reviewed Jul 21, 2023

View reviewed changes

Comment thread pulsar-cli-utils/pom.xml Outdated

Technoboy- added this to the 3.2.0 milestone Jul 31, 2023

Merge remote-tracking branch 'upstream/master' into pip280-separate-m…

238b590

…odule

gaoran10 reviewed Aug 4, 2023

View reviewed changes

Comment thread pulsar-cli-utils/src/main/java/org/apache/pulsar/cli/converters/RelativeTimeUtil.java Outdated

gaoran10 approved these changes Aug 4, 2023

View reviewed changes

Remove whitespace

f0f9f9c

Co-authored-by: ran <gaoran_10@126.com>

BewareMyPower reviewed Aug 4, 2023

View reviewed changes

Comment thread pulsar-cli-utils/src/main/java/org/apache/pulsar/cli/converters/RelativeTimeUtil.java

Add more test covreatge and reformat

a9eb0d5

JooHyukKim requested a review from BewareMyPower August 5, 2023 00:03

JooHyukKim added 3 commits August 12, 2023 21:06

Merge remote-tracking branch 'upstream/master' into pip280-separate-m…

44dc72f

…odule

Remove unncessary comment

e0ad3bf

Merge remote-tracking branch 'upstream/master' into pip280-separate-m…

b4b0ba7

…odule

BewareMyPower approved these changes Aug 16, 2023

View reviewed changes

tisonkun approved these changes Aug 23, 2023

View reviewed changes

tisonkun merged commit 4f9b199 into apache:master Aug 23, 2023

JooHyukKim deleted the pip280-separate-module branch August 23, 2023 12:16

This was referenced Aug 23, 2023

[refactor][cli][PIP-280] Tracker : Refactor byte, time parsing logic using new pulsar-cli-utils module #21056

Closed

[refactor][cli][PIP-280] Add missing pulsar-cli-utils module registration in Maven core-modules profile #21062

Merged

lhotari mentioned this pull request Feb 28, 2024

[Bug] pulsar-admin set-retention reports an invalid retention policy when size != -1 #22138

Closed

2 tasks

thetumbled pushed a commit to thetumbled/pulsar that referenced this pull request Feb 28, 2024

[refactor][cli] [PIP-280] Create new pulsar-cli-utils module (apache#…

e558bb3

…20782) Co-authored-by: ran <gaoran_10@126.com> (cherry picked from commit 4f9b199)

This was referenced Feb 28, 2024

[cherry-pick] [branch-3.0] Fix getting last message ID when there are ongoing transactions #22130

Closed

[fix] [branch-3.0] fix compile error in branch 3.0 #22143

Closed

thetumbled pushed a commit to thetumbled/pulsar that referenced this pull request Feb 28, 2024

[refactor][cli] [PIP-280] Create new pulsar-cli-utils module (apache#…

7639c66

…20782) Co-authored-by: ran <gaoran_10@126.com> (cherry picked from commit 4f9b199)

thetumbled pushed a commit to thetumbled/pulsar that referenced this pull request Feb 28, 2024

[refactor][cli] [PIP-280] Create new pulsar-cli-utils module (apache#…

265775f

…20782) Co-authored-by: ran <gaoran_10@126.com> (cherry picked from commit 4f9b199)

heesung-sohn mentioned this pull request Feb 28, 2024

[improve][admin] Expose the offload threshold in seconds to the admin #22101

Merged

15 tasks

lhotari mentioned this pull request Dec 20, 2024

[fix][cli] Fix set-retention with >2GB size value for topic policy #23689

Merged

4 tasks

Conversation

JooHyukKim commented Jul 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Note

Verifying this change

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

Documentation

Matching PR in forked repository

Uh oh!

codecov-commenter commented Jul 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JooHyukKim commented Jul 12, 2023

Uh oh!

JooHyukKim commented Jul 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JooHyukKim commented Jul 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JooHyukKim commented Jul 16, 2023

Uh oh!

JooHyukKim commented Jul 19, 2023

Uh oh!

JooHyukKim commented Jul 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tisonkun commented Jul 20, 2023

Uh oh!

Uh oh!

Uh oh!

gaoran10 left a comment

Choose a reason for hiding this comment

Uh oh!

BewareMyPower left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JooHyukKim commented Aug 16, 2023

Uh oh!

JooHyukKim commented Aug 17, 2023

Uh oh!

tisonkun left a comment

Choose a reason for hiding this comment

Uh oh!

tisonkun commented Aug 23, 2023

Uh oh!

JooHyukKim commented Aug 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

JooHyukKim commented Jul 11, 2023 •

edited

Loading

codecov-commenter commented Jul 12, 2023 •

edited

Loading

JooHyukKim commented Jul 12, 2023 •

edited

Loading

JooHyukKim commented Jul 13, 2023 •

edited

Loading

JooHyukKim commented Jul 19, 2023 •

edited

Loading