Nice to have: notRetryOn() and retryOn() in RetryTemplateBuilder

[hotire]

Summary

I noted this while using the RetryTemplateBuilder.

https://github.com/spring-projects/spring-retry/blob/main/src/main/java/org/springframework/retry/support/RetryTemplateBuilder.java#L285

It would be nice to support list parameter like RetryTopicConfigurationBuilder in spring-kafka

https://github.com/spring-projects/spring-kafka/blob/main/spring-kafka/src/main/java/org/springframework/kafka/retrytopic/RetryTopicConfigurationBuilder.java#L308

Sample

public RetryTemplateBuilder notRetryOn(List<Class<? extends Throwable>> throwables) {
       for (final Class<? extends Throwable> throwable : throwables) {
		classifierBuilder().notRetryOn(throwable);
	}
	return this;
}

public RetryTemplateBuilder retryOn(List<Class<? extends Throwable>> throwables) {
	for (final Class<? extends Throwable> throwable : throwables) {
		classifierBuilder().retryOn(throwable);
	}
	return this;
}

[artembilan]

As @garyrussell has marked this issue , the contribution is welcome!

While looking into this, I wonder why list, why not varargs?
Just classes, so would be much easier to list them over the comma.
I believe user are going to do List.of() for this contract anyway. So, let's make their life easier with varargs!

[hotire]

Using varargs is much simpler, but with a warning message...

"Unchecked generics array creation for varargs parameter"

[artembilan]

That's true.
Just add this to those varargs methods:

@SuppressWarnings("varargs")
@SafeVarargs

And the warn is gone.

[hotire]

@artembilan

That's a cool idea.!!

Let's change from List to varargs.

[artembilan]

Although it might be a breaking change for the existing notRetryOn(Class<? extends Throwable> throwable).
It might be OK for those projects which are going to upgrade and compile. But those with just drop-in replacement are going to suffer from non-compatible bytecode...

[hotire]

SafeVarargs annotation is API available from 1.7 or later.... 😂

[artembilan]

Yeah... As long as I remember the @SuppressWarnings("varargs") i enough to suppress it in the build logs.
Plus annotations works the way that they are ignored if they are not present in the classpath.
Anyway: let's stick with a list for now.
Please, bring your PR back and sorry for a false noise from me: we will revise it into varargs when we switch to version 2.0.

Thank you!

[dratler]

Hi @garyrussell , @artembilan is this open for the greb ?

[hotire]

Thanks for the great ideas.!!

[artembilan]

Hi @dratler !

Thank you for the offer, but looks like @hotire has taken care about this already.
See his PR #255

You can take any other issue in backlog.