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

[fix][broker] Fix geo-replication admin client url #22584

Conversation

Demogorgon314
Copy link
Member

Motivation

When we only set the TLS URLs for ClusterData and set the brokerClientTlsEnabled to true, the admin client for geo-replication will be failed to create because the else if block(StringUtils.isEmpty(data.getServiceUrl())) logic is wrong.

Modifications

  • Allow the cluster data only set tls url

Documentation

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

@Demogorgon314 Demogorgon314 self-assigned this Apr 25, 2024
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Apr 25, 2024
@nodece nodece closed this Apr 28, 2024
@nodece nodece reopened this Apr 28, 2024
@Demogorgon314 Demogorgon314 force-pushed the Demogorgon314/fix-geo-replication-admin-url branch from 7deccc7 to f2a5af4 Compare May 8, 2024 12:15
@nodece
Copy link
Member

nodece commented May 9, 2024

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 33.33333% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 73.13%. Comparing base (bbc6224) to head (6b0dc19).
Report is 246 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #22584      +/-   ##
============================================
- Coverage     73.57%   73.13%   -0.45%     
- Complexity    32624    32772     +148     
============================================
  Files          1877     1887      +10     
  Lines        139502   141073    +1571     
  Branches      15299    15484     +185     
============================================
+ Hits         102638   103168     +530     
- Misses        28908    29949    +1041     
  Partials       7956     7956              
Flag Coverage Δ
inttests 27.32% <0.00%> (+2.73%) ⬆️
systests 24.58% <0.00%> (+0.25%) ⬆️
unittests 72.15% <33.33%> (-0.70%) ⬇️

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

Files Coverage Δ
...rg/apache/pulsar/broker/service/BrokerService.java 81.45% <33.33%> (+0.67%) ⬆️

... and 340 files with indirect coverage changes

@nodece nodece merged commit bd4c57d into apache:master May 9, 2024
50 checks passed
@nodece nodece added this to the 3.4.0 milestone May 9, 2024
poorbarcode pushed a commit that referenced this pull request Jul 13, 2024
poorbarcode pushed a commit that referenced this pull request Jul 13, 2024
poorbarcode pushed a commit that referenced this pull request Jul 13, 2024
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 14, 2024
(cherry picked from commit bd4c57d)
(cherry picked from commit 3ccc213)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 15, 2024
(cherry picked from commit bd4c57d)
(cherry picked from commit 3ccc213)
nodece pushed a commit that referenced this pull request Jul 23, 2024
nodece pushed a commit to ascentstream/pulsar that referenced this pull request Jul 24, 2024
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.

7 participants