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.

Sign up for GitHub

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

Jump to bottom

Add configuration properties for cluster-level failover with Apache Pulsar #38559

Closed
wants to merge 5 commits into from
Closed

Add configuration properties for cluster-level failover with Apache Pulsar #38559

wants to merge 5 commits into from

Conversation

swamymavuri
Copy link
Contributor

No description provided.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 27, 2023
@swamymavuri swamymavuri mentioned this pull request Nov 27, 2023
Closed
@wilkinsona wilkinsona changed the title #ApachePulsar-375-Add Support for AutoClusterFailover Provide configuration properties for cluster-level failover with Apache Pulsar Nov 27, 2023
onobc
onobc reviewed Nov 27, 2023
View reviewed changes
Copy link
Contributor

@onobc onobc left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @swamymavuri. I have added a few comments/suggestions but overall this LGTM. This part of Spring Pulsar lives in Spring Boot so someone from the Boot team will also need to review.

I don't think the docs as they are intentionally high level.

Also, I noticed you are working from your main branch in your fork. We typically work from a userbranch in our fork.

...oconfigure/src/main/java/org/springframework/boot/autoconfigure/pulsar/PulsarProperties.java Outdated Show resolved Hide resolved
...gure/src/main/java/org/springframework/boot/autoconfigure/pulsar/PulsarPropertiesMapper.java Outdated Show resolved Hide resolved
...gure/src/main/java/org/springframework/boot/autoconfigure/pulsar/PulsarPropertiesMapper.java Outdated Show resolved Hide resolved
...re/src/test/java/org/springframework/boot/autoconfigure/pulsar/PulsarConfigurationTests.java Show resolved Hide resolved
...re/src/test/java/org/springframework/boot/autoconfigure/pulsar/PulsarConfigurationTests.java Outdated Show resolved Hide resolved
...gure/src/main/java/org/springframework/boot/autoconfigure/pulsar/PulsarPropertiesMapper.java Outdated Show resolved Hide resolved
@swamymavuri
Copy link
Contributor Author

Thanks for the review comments @onobc ,
Updated the code as per review comments, please take a look once

Also can you suggest

  1. whether do i need to raise a PR from my fork to your Branch(fork --> your branch --> Spring boot Main Branch), or for this feature, we can leave it as it is(from my fork -> Spring Boot Main Branch).
  2. Do we need to reach out any one for review this PR from spring boot
    Thanks

@onobc
Copy link
Contributor

onobc commented Dec 1, 2023

Also can you suggest

whether do i need to raise a PR from my fork to your Branch(fork --> your branch --> Spring boot Main Branch), or for this
feature, we can leave it as it is(from my fork -> Spring Boot Main Branch).

We typically follow the fork-and-branch-git-workflow. The only difference w/ yours is that instead of doing your changes in origin/main you would create a userbranch such as origin/gh-38559-pulsar-auto-failover (or whatever name you choose) and then use that to submit the PR. Whatever you do, do not resubmit an entire new PR as that will cause a bunch of noise and we would lose the current comments etc.. @philwebb what is the preferred action in this case?

Do we need to reach out any one for review this PR from spring boot

Nope. Trust me, the team sees the PR and are just very busy - but they will get to the PR.

@mhalbritter mhalbritter added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 10, 2024
@mhalbritter mhalbritter added this to the 3.x milestone Jan 10, 2024
@mhalbritter mhalbritter changed the title Provide configuration properties for cluster-level failover with Apache Pulsar Add configuration properties for cluster-level failover with Apache Pulsar Jan 10, 2024
@mhalbritter mhalbritter self-assigned this Jan 10, 2024
mhalbritter pushed a commit that referenced this pull request Jan 10, 2024
@swamymavuri @mhalbritter
Add support for Pulsar cluster-level failover
c3e3372 
See gh-38559
mhalbritter added a commit that referenced this pull request Jan 10, 2024
@mhalbritter
Polish "Add support for Pulsar cluster-level failover"
f696190 
See gh-38559
@mhalbritter
Copy link
Contributor

Thank you very much and congratulations on your first contribution 🎉!

@mhalbritter mhalbritter modified the milestones: 3.x, 3.3.0-M1 Jan 10, 2024
@swamymavuri
Copy link
Contributor Author

Thanks @mhalbritter

izeye added a commit to izeye/spring-boot that referenced this pull request Mar 13, 2024
@izeye
Polish spring-projectsgh-38559
80b80fc
@izeye izeye mentioned this pull request Mar 13, 2024
Closed
mhalbritter pushed a commit that referenced this pull request Mar 14, 2024
@izeye @mhalbritter
Polish gh-38559
c5df4e3 
See gh-39931
mhalbritter added a commit that referenced this pull request Mar 14, 2024
@mhalbritter
Merge pull request #39931 from izeye
5600a02 
* pr/39931:
  Polish gh-38559

Closes gh-39931
@wilkinsona wilkinsona mentioned this pull request Mar 22, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@onobc onobc onobc left review comments

Assignees

@mhalbritter mhalbritter

Labels
type: enhancement A general enhancement
Projects
None yet
Milestone
3.3.0-M1
Development

Successfully merging this pull request may close these issues.
4 participants
@swamymavuri @onobc @mhalbritter @spring-projects-issues