Make RestTemplate retry opt out.

[tkvangorder]

We have recently moved to Camden.SR6 and converted our code over to use the new retry functionality in Camden (We were using the deprecated Netflix Http client). One thing that tripped us up initially was that the rest client still requires us to "opt in" to the retry logic via the property spring.cloud.loadbalancer.retry.enabled=true. It would be helpful to either change this, such that this property is defaulted true (and you need to opt-out if you don't want retry) or to add something to the documentation that clarifies that this property must be set to enable retry within the RestTemplate.

I think it would make more sense to default this to true to keep it consistent with Zuul, which has a property to opt-out of retry logic.

Everything is working great, once we figured out we needed to enable the new functionality.

[spencergibb]

So, as far as I can tell, spring.cloud.loadbalancer.retry.enabled is only required for a loadbalanced RestTemplate. All other retry is conditional on spring-retry on the classpath.

[tkvangorder]

Right,

Feign == Will retry, if spring-retry is on your classpath. Zuul == Will retry, if spring-retry is on your classpath. RestTemplate == Will retry, if spring-retry is on your classpath, you annotate your RestTemplate with @LoadBalanced, AND you have set "spring.cloud.loadbalancer.retry.enabled" to true.

You already have the ability to opt-in to use a load balancer via the annotation. Wouldn't it be more consistent to automatically include the retry logic for any LoadBalanced rest template if spring-retry is on your classpath?

So my suggestion was one of three things:

• Change the default value of spring.cloud.loadbalancer.retry.enabled to true. Leaving this attribute in place would still give the developer the ability to opt-out. This would be consistent with how Feign and Zuul work.

• Just add a note to the documentation that load-balanced RestTemplates must include the spring-retry library AND must opt-in via the spring.cloud.loadbalancer.retry.enabled to use retry.

• Add an optional attribute to the @LoadBalanced Annotation to specify re-try policy on a per-template basis. I still like the idea of defaulting this to to "true". This third option would obviously only work if the spring-retry library were also on the class path.

@LoadBalaced(retryEnabled=true)

[spencergibb]

I think the docs are missing the 'spring-retry' dependency with the property. @ryanjbaxter I think this is fine to make the default in Dalston, do you?

[ryanjbaxter]

Sorry about forgetting to document the property. I agree lets make the default true for Dalston. Do we just want to document the property for Camden then?

[spencergibb]

It's documented, but is missing the spring retry dependency, but yes.

[ryanjbaxter]

there is a"bug" in the Camden version, it is missing the .enabled I will fix both issues

[spencergibb]

Looks like the docs are done, just changing the default, correct @ryanjbaxter?