-
Notifications
You must be signed in to change notification settings - Fork 1.1k
GH-10345: Migrate Kafka module to Core Retry #10387
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
Conversation
Related to: spring-projects#10345 * Replace `RetryTemplate` from Spring Retry to `RetryTemplate` from Spring Core since Spring Retry is at its sunset. There is no deprecation for Spring Retry API usage since the idea is to remove Spring Retry dependency altogether for the upcoming major for all projects in the Spring portfolio * Introduce a `KafkaInboundEndpoint.RetryContext` as an `AttributeAccessorSupport` extension to track the number of retries for functional compatibility
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM just some nitpicks as usual ;-)
* @param consumer the consumer. | ||
* @param runnable the runnable. | ||
*/ | ||
default void doWithRetry(RetryTemplate template, @Nullable RecoveryCallback<?> callback, ConsumerRecord<?, ?> record, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't doWithRetry be @Nullable
since it can return a null?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return does not apply to the logic of Kafka Inbound Endpoints.
Therefore void
here and just swallowing null
return from the RetryTemplate.execute()
.
...n-kafka/src/main/java/org/springframework/integration/kafka/inbound/KafkaInboundGateway.java
Show resolved
Hide resolved
...ain/java/org/springframework/integration/kafka/inbound/KafkaMessageDrivenChannelAdapter.java
Show resolved
Hide resolved
* Remove redundant now `// NOSONAR` comments from the `KafkaInboundGateway` & `KafkaMessageDrivenChannelAdapter` * Fix missed in the previous commit `KafkaMessageDrivenChannelAdapter.setAttributesIfNecessary()` * Change the logic around `KafkaMessageDrivenChannelAdapter.sendMessageIfAny()` to the `doSendMessage()` to reflect its reality * Change conversion failure `debug` logic to `warn`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Good stuff 👍👍 |
Related to: #10345
Replace
RetryTemplate
from Spring Retry toRetryTemplate
from Spring Core since Spring Retry is at its sunset.There is no deprecation for Spring Retry API usage since the idea is to remove Spring Retry dependency altogether for the upcoming major for all projects in the Spring portfolio
Introduce a
KafkaInboundEndpoint.RetryContext
as anAttributeAccessorSupport
extension to track the number of retries for functional compatibility