Skip to content

Conversation

@zjxxzjwang
Copy link
Contributor

@zjxxzjwang zjxxzjwang commented Nov 21, 2024

Motivation

Topic name rule

  • old version: {tenant}/{cluster}/{namespace}/{topic}

  • new version: {tenant}/{namespace}/{topic}

If the name of an automatically created topic contains a "/", the topic will be resolved into a four-segment format (containing a layer of cluster directories). As a result, the automatically created topic does not conform to the latest topic naming rules

Modifications

No allowto automatically create topic names that contain slashes

Documentation

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

Matching PR in forked repository

PR in forked repository:

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Nov 21, 2024
@lhotari lhotari changed the title No allowto automatically create topic names that contain slashes [feat][broker] Do not automatically create topics using legacy naming scheme that contains the cluster Nov 21, 2024
@lhotari
Copy link
Member

lhotari commented Nov 21, 2024

Related to #23594

@lhotari lhotari changed the title [feat][broker] Do not automatically create topics using legacy naming scheme that contains the cluster [feat][broker] Prevent auto-creation of topics using legacy cluster-based naming scheme Nov 22, 2024
@zjxxzjwang zjxxzjwang requested a review from Jason918 December 3, 2024 14:51
Copy link
Contributor

@Jason918 Jason918 left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-commenter
Copy link

codecov-commenter commented Dec 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.16%. Comparing base (bbc6224) to head (9cac6f9).
Report is 821 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #23620      +/-   ##
============================================
+ Coverage     73.57%   74.16%   +0.59%     
+ Complexity    32624    32153     -471     
============================================
  Files          1877     1853      -24     
  Lines        139502   143385    +3883     
  Branches      15299    16279     +980     
============================================
+ Hits         102638   106343    +3705     
+ Misses        28908    28687     -221     
- Partials       7956     8355     +399     
Flag Coverage Δ
inttests 26.71% <75.00%> (+2.13%) ⬆️
systests 23.13% <50.00%> (-1.19%) ⬇️
unittests 73.69% <100.00%> (+0.84%) ⬆️

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

Files with missing lines Coverage Δ
...org/apache/pulsar/broker/ServiceConfiguration.java 98.14% <100.00%> (-1.25%) ⬇️
...rg/apache/pulsar/broker/service/BrokerService.java 84.88% <100.00%> (+4.10%) ⬆️

... and 1023 files with indirect coverage changes

@Jason918 Jason918 merged commit b02d52c into apache:master Jan 2, 2025
52 checks passed
hanmz pushed a commit to hanmz/pulsar that referenced this pull request Feb 12, 2025
…ased naming scheme (apache#23620)

Co-authored-by: zjxxzjwang <zjxxzjwang@tencent.com>
@BewareMyPower BewareMyPower added this to the 4.1.0 milestone Feb 26, 2025
@BewareMyPower
Copy link
Contributor

This PR introduced a new config without any PIP. Currently it's not cherry-picked into any existing branch. But we should consider adding a PIP for it.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants