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

Use KafkaListenerEndpoint instead of MethodKafkaListenerEndpoint in the RetryTopicNamesProvider #2312

Merged
merged 1 commit into from
Jun 22, 2022

Conversation

filiphr
Copy link
Contributor

@filiphr filiphr commented Jun 21, 2022

Fixes #2294

@tomazfernandes
Copy link
Contributor

In my understanding this would be a breaking change. Since the feature is marked as experimental API, perhaps this small change wouldn't be much of a problem, maybe for 2.9 which is an opt-in version anyway.

Other than that, I'm ok with this.

Thanks.

Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

This is breaking change just on the byte code level.
From the API you won't see a difference calling this methods.

However this is just an improvement, so it goes only to 2.9.x and 3.0. not back-porting to 2.8.x - not critical.

I'll defer merging to @garyrussell .

Thank you all for the contribution into this issue!

@garyrussell garyrussell merged commit b57c608 into spring-projects:main Jun 22, 2022
@filiphr
Copy link
Contributor Author

filiphr commented Jun 22, 2022

This is breaking change just on the byte code level.

Just FYI, this is a breaking change if someone has implemented the interface. I am of course OK in this going into 2.9, because we can then update our things and simplify our code. However, I wanted to give a heads up

@filiphr filiphr deleted the issue/2294 branch June 22, 2022 15:10
@filiphr
Copy link
Contributor Author

filiphr commented Jun 22, 2022

Thanks for integrating this into main. It is really appreciated

@garyrussell
Copy link
Contributor

Cherry-picked to 2.9.x as 78fa932

@filiphr In future, please limit commit headlines to 50 chars (see contribution guidelines).

@filiphr
Copy link
Contributor Author

filiphr commented Jun 22, 2022

@filiphr In future, please limit commit headlines to 50 chars (see contribution guidelines).

Sorry for that. Next time I'll limit the commit to 50 chars. I thought you might also do a squash and merge and adapt the headline to what you think makes most sense to you 😄

@garyrussell
Copy link
Contributor

Yeah - unfortunately GH added these very convenient buttons to merge directly and I only. noticed the long headline when I pulled it locally; not a big deal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RetryTopicNamesProvider depends on an impl class instead of KafkaListenerEndpoint
4 participants