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

RabbitMQ - RetryTemplate configuration hook #11683

Closed
wants to merge 1 commit into from

Conversation

garyrussell
Copy link
Contributor

Provide a mechanism so that users can further configure RetryTemplates
used in RabbitTemplate and listeners.

An example would be to add a retry listener so the user can monitor/log
retry attempts.

https://stackoverflow.com/questions/48331502/logging-exceptions-thrown-by-message-listeners-for-spring-amqp/48332001#48332001

Or, to set a custom RetryContextCache.

Provide a mechanism so that users can further configure `RetryTemplate`s
used in `RabbitTemplate` and listeners.

An example would be to add a retry listener so the user can monitor/log
retry attempts.

https://stackoverflow.com/questions/48331502/logging-exceptions-thrown-by-message-listeners-for-spring-amqp/48332001#48332001

Or, to set a custom `RetryContextCache`.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 19, 2018
Copy link
Member

@snicoll snicoll left a comment

Choose a reason for hiding this comment

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

I am not sure I understand the rationale of the PR yet. It looks a bit "off" compared what we're used to do.

I certainly don't like that a public contract takes the properties object.

* @since 2.0
*
*/
public interface RetryTemplateConfigurer<P> {
Copy link
Member

Choose a reason for hiding this comment

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

We usually use the term Customizer for this.

* @since 2.0
*
*/
public class RabbitTemplateRetryTemplateConfigurer extends RabbitRetryTemplateConfigurer {
Copy link
Member

Choose a reason for hiding this comment

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

This empty class seems suspicious.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's an empty class because there are two RetryTemplates - one for the consumers and one for the template.

How else can I determine that the user has provided a customizer for one Vs. the other?

* @param template the template.
* @param properties the properties.
*/
void configure(RetryTemplate template, P properties);
Copy link
Member

Choose a reason for hiding this comment

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

There is a mixed of concept here. Usually a customizer is something that applies something extra. It doesn't get the configuration properties object in argument (as it is an auto-configuration concern).

}
RetryTemplate template = new RetryTemplate();
retryConfigurer.configure(template, retryConfig);
builder.retryOperations(template);
Copy link
Member

Choose a reason for hiding this comment

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

This code doesn't is not equivalent right? RabbitListenerRetryTemplateConfigurer() does more than what you removed as far as I see.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's exactly equivalent; just that the old code used a shortcut on the builder.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Jan 19, 2018
@garyrussell
Copy link
Contributor Author

Closed; replaced by #11697

@snicoll snicoll added status: duplicate A duplicate of another issue and removed status: waiting-for-triage An issue we've not yet triaged status: waiting-for-feedback We need additional information before we can continue labels Jan 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate A duplicate of another issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants