search

spring-cloud / spring-cloud-commons

Common classes used in different Spring Cloud implementations
Apache License 2.0
701 stars 697 forks source link

Allow building @LoadBalanced RestClient in component constructor. #1339

Closed OlgaMaciaszek closed 6 months ago

OlgaMaciaszek commented 6 months ago

Load balanced RestClient with deferring postprocessor.

OlgaMaciaszek commented 6 months ago

I'm really sorry for the mess around that deprecation @wind57. In fact, it was being load-balanced with the previous change, however, with the caveat that a component constructor would be to early to build the RestClient instance with a @LoadBalanced RestClient.Builder, which is why it's been reworked again.

wind57 commented 6 months ago

My point was rather different, I'm sorry for not being verbose enough.

Before this PR, I saw that LoadBalancerRestClientBuilderBeanPostProcessor that was a @Bean, was removed to be such and deprecated. Now, the question I had, was how is it possible then to find out who is actually annotated with @LoadBalanced, if not a BeanPostProcessor exists?

Yes, you did add SmartInitializingSingleton and a customizer, but doesn't that mean that load-balancing will be applied to every instance of RestClinet.Builder, be that annotated or not with @LoadBalanced, simply because its done for everyone. There is no logical check of "if bean is annotated with @LoadBalanced" : do that...

I hope I make more sense now. And don't worry about the issue at all!

OlgaMaciaszek commented 6 months ago

The @LoadBalanced annotation over in: @LoadBalanced @Autowired(required = false) private List<RestClient.Builder> restClientBuilders = Collections.emptyList(); works as a qualifier; that's also how it's always been done for RestTemplate, just too early for RestClient for some of the scenarios, since there we work with builder instances and not actual client instances.

wind57 commented 6 months ago

oh my, I'm an idiot, you're right! I completely missed the @Qualifier in @LoadBalanced. Thank you for taking the time to explain.

OlgaMaciaszek commented 6 months ago

No worries, it's not very straightforward.