Skip to content

Conversation

hanmz
Copy link
Contributor

@hanmz hanmz commented Oct 21, 2024

Motivation

Currently, we can create a non-persistent partition topic contain -partition-. However, we cannot use it as a normal non-persistent partition topic. For example, when we use the partitioned-stats command, we will encounter errors.

Partitioned Topic Name should not contain '-partition-'
Reason: Partitioned Topic Name should not contain '-partition-'

Modifications

We should disable the creation of non-persistent partition topics contain -partition-.

Documentation

Verifying this change

All unit tests passed, not have any behavior change.

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

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Oct 21, 2024
@lhotari
Copy link
Member

lhotari commented Oct 21, 2024

Thanks for the contribution, @hanmz. Would you be able to add a test case to cover this change?

@lhotari lhotari changed the title [fix][broker]Not allow to create non-persistent partitioned topic if topic name contain '-partition-'. [fix][broker] Don't allow creating a non-persistent partitioned topic with '-partition-' in name Oct 21, 2024
@hanmz
Copy link
Contributor Author

hanmz commented Oct 28, 2024

Thanks for the contribution, @hanmz. Would you be able to add a test case to cover this change?

OK, I've added a test case.

@Technoboy- Technoboy- added this to the 4.1.0 milestone Oct 29, 2024
@codecov-commenter
Copy link

codecov-commenter commented Oct 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 40.75%. Comparing base (bbc6224) to head (fbe1f32).
Report is 793 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (bbc6224) and HEAD (fbe1f32). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (bbc6224) HEAD (fbe1f32)
unittests 2 1
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #23488       +/-   ##
=============================================
- Coverage     73.57%   40.75%   -32.83%     
+ Complexity    32624     1417    -31207     
=============================================
  Files          1877     1805       -72     
  Lines        139502   148024     +8522     
  Branches      15299    17116     +1817     
=============================================
- Hits         102638    60322    -42316     
- Misses        28908    80090    +51182     
+ Partials       7956     7612      -344     
Flag Coverage Δ
inttests 27.24% <100.00%> (+2.66%) ⬆️
systests 24.34% <0.00%> (+0.01%) ⬆️
unittests 36.03% <100.00%> (-36.81%) ⬇️

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

Files with missing lines Coverage Δ
...he/pulsar/broker/admin/v2/NonPersistentTopics.java 29.20% <100.00%> (-32.27%) ⬇️

... and 1577 files with indirect coverage changes

@lhotari
Copy link
Member

lhotari commented Nov 5, 2024

Closing and reopening to trigger a new CI run

@lhotari lhotari closed this Nov 5, 2024
@lhotari lhotari reopened this Nov 5, 2024
@hanmz
Copy link
Contributor Author

hanmz commented Dec 9, 2024

/pulsarbot rerun-failure-checks

@Jason918
Copy link
Contributor

reopen to trigger ci

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-not-needed Your PR changes do not impact docs release/4.0.8
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants