Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[improve][broker] Make cluster metadata init command support metadata config path #23269

Conversation

Demogorgon314
Copy link
Member

Motivation

The pulsar configuration supported metadataStoreConfigPath and configurationStoreConfigPath, but the cluster metadata init command does not have a place to set the config path.

Modifications

Make cluster metadata init command support metadata config path

Documentation

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

@Demogorgon314 Demogorgon314 self-assigned this Sep 9, 2024
@github-actions github-actions bot added the doc-required Your PR changes impact docs and you will update later. label Sep 9, 2024
Copy link
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, left comments about keeping binary compatibility

Copy link
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

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Project coverage is 74.51%. Comparing base (bbc6224) to head (2b8eaf5).
Report is 567 commits behind head on master.

Files with missing lines Patch % Lines
.../org/apache/pulsar/PulsarClusterMetadataSetup.java 80.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #23269      +/-   ##
============================================
+ Coverage     73.57%   74.51%   +0.93%     
- Complexity    32624    33683    +1059     
============================================
  Files          1877     1926      +49     
  Lines        139502   145057    +5555     
  Branches      15299    15864     +565     
============================================
+ Hits         102638   108088    +5450     
+ Misses        28908    28700     -208     
- Partials       7956     8269     +313     
Flag Coverage Δ
inttests 27.93% <28.57%> (+3.35%) ⬆️
systests 24.66% <0.00%> (+0.34%) ⬆️
unittests 73.88% <85.71%> (+1.03%) ⬆️

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

Files with missing lines Coverage Δ
...org/apache/pulsar/PulsarInitialNamespaceSetup.java 73.80% <100.00%> (-1.20%) ⬇️
...sar/PulsarTransactionCoordinatorMetadataSetup.java 61.11% <100.00%> (-2.05%) ⬇️
.../org/apache/pulsar/PulsarClusterMetadataSetup.java 72.43% <80.00%> (-0.09%) ⬇️

... and 549 files with indirect coverage changes

@lhotari
Copy link
Member

lhotari commented Sep 9, 2024

It seems that this is related to PIP-366 #23033 and should go only in master branch?

@lhotari lhotari merged commit 46f99b9 into apache:master Sep 9, 2024
51 checks passed
michalcukierman pushed a commit to michalcukierman/pulsar that referenced this pull request Sep 11, 2024
Demogorgon314 added a commit that referenced this pull request Oct 12, 2024
@lhotari lhotari added this to the 4.0.0 milestone Oct 14, 2024
lhotari pushed a commit that referenced this pull request Oct 21, 2024
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Oct 28, 2024
… config path (apache#23269)

(cherry picked from commit 46f99b9)
(cherry picked from commit fa2e7d8)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Nov 6, 2024
… config path (apache#23269)

(cherry picked from commit 46f99b9)
(cherry picked from commit fa2e7d8)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Nov 6, 2024
… config path (apache#23269)

(cherry picked from commit 46f99b9)
(cherry picked from commit fa2e7d8)
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.

4 participants