Skip to content

[fix][cli] Fix the bug when set-retention specified size with -T#22150

Merged
lhotari merged 1 commit intoapache:masterfrom
Technoboy-:fix-22138
Feb 28, 2024
Merged

[fix][cli] Fix the bug when set-retention specified size with -T#22150
lhotari merged 1 commit intoapache:masterfrom
Technoboy-:fix-22138

Conversation

@Technoboy-
Copy link
Copy Markdown
Contributor

Fixes #22138

Motivation

When specified with -s with -xT, it will overflow when convert long to int.
image
image

Documentation

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

@Technoboy- Technoboy- self-assigned this Feb 28, 2024
@Technoboy- Technoboy- added this to the 3.3.0 milestone Feb 28, 2024
@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Feb 28, 2024
Copy link
Copy Markdown
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Good work @Technoboy-

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Feb 28, 2024

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 73.54%. Comparing base (bbc6224) to head (c3fd693).
⚠️ Report is 1247 commits behind head on master.

Files with missing lines Patch % Lines
...ava/org/apache/pulsar/admin/cli/CmdNamespaces.java 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #22150      +/-   ##
============================================
- Coverage     73.57%   73.54%   -0.04%     
+ Complexity    32624    32116     -508     
============================================
  Files          1877     1877              
  Lines        139502   139546      +44     
  Branches      15299    15312      +13     
============================================
- Hits         102638   102627      -11     
- Misses        28908    28954      +46     
- Partials       7956     7965       +9     
Flag Coverage Δ
inttests 24.65% <0.00%> (+0.07%) ⬆️
systests 24.24% <0.00%> (-0.08%) ⬇️
unittests 72.79% <50.00%> (-0.05%) ⬇️

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

Files with missing lines Coverage Δ
...ava/org/apache/pulsar/admin/cli/CmdNamespaces.java 78.73% <50.00%> (ø)

... and 55 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.

@lhotari
Copy link
Copy Markdown
Member

lhotari commented Dec 6, 2024

There was a similar bug in pulsar-admin topics set-retention and pulsar-admin topicPolicies set-retention, fixed by #23689, please review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

6 participants