spring-cloud-openfeign icon indicating copy to clipboard operation
spring-cloud-openfeign copied to clipboard

Support lb:// url in FeingClient annotation

Open antechrestos opened this issue 4 years ago • 8 comments

Current behaviour

In a project where is used spring cloud gateway, feign client (with target put in configuration), we ended up having feign clients declared as follows

@FeignClient(name="${service.name:any-name}", url="${service.direct-url:}")

With configuration as follows

service:
   name: service-id
   # url: http://direct-url (only used when service discovery is not available)

This way we are able, by configuration, to either run in service discovery/load balancing mode and direct mode.

Proposal

I kind of like the way spring cloud gateway handle that by allowing lb:// protocol. Allowing this protocol in url attribute would allow the following implementation

@FeignClient(name="technical-name", url="${service.uri}")

with following configuration

service:
   uri: "lb://service-id" # or "http//direct-url"

this way it will let common behaviour, allowing to have a common behaviour in url specification.

I spotted the code responsible for feign client building and think that it would be easily doable by

  • only appending http:// if url attribute value does not match the [a-z]+://.* pattern
  • remove load balancing client if and only if url attribute does not starts with lb://

I am eager to implement it if you allow me to do so. I am also pleased to discuss about it and about any alternative I missed.

Thank you

antechrestos avatar Feb 20 '21 19:02 antechrestos

@OlgaMaciaszek what the point on this issue? Can I make a PR?

antechrestos avatar Mar 18 '21 15:03 antechrestos

PR submitted

antechrestos avatar Mar 30 '21 17:03 antechrestos

@antechrestos Actually, I am not sure this is a change we want to make. We are going to see if there's interest in the community and, if yes, then, we will discuss it in the team.

OlgaMaciaszek avatar Apr 12 '21 10:04 OlgaMaciaszek

@OlgaMaciaszek Agreed. Just for clarification, is there any documentation describing "the waiting for votes" adhesion and what is the comunity? Is there a number of :+1: expected on the issue? Or is it based on an adhesion of an inner community (spring developers for instance)? Thank you

antechrestos avatar Apr 12 '21 11:04 antechrestos

We check how many users request a feature. If we see considerable interest, then we discuss the possible issues and implications within the Spring Cloud team, taking into account the discussion in the issue and other considerations related to maintenance, planned future project development and possible issues.

OlgaMaciaszek avatar Apr 12 '21 12:04 OlgaMaciaszek

@OlgaMaciaszek ok that's more clear

antechrestos avatar Apr 12 '21 12:04 antechrestos

To my mind, I think the real question is if there is value to apply the concept of lb:// url used in spring cloud gateway in other spring projects or if it needs to be restricted to it.

As an alternative, it may be useful to let the user decide whether (s)he wants to skip load balancing/discovery behaviour. Because currently, the url use hides the fact that we force the use of delegated client... What do you think of it?

antechrestos avatar Apr 12 '21 12:04 antechrestos

@OlgaMaciaszek do you think I should open a new issue proposing to handle load-balancing/discovery in a dedicated properties rather than hidden behind url attribute?

antechrestos avatar Apr 13 '21 08:04 antechrestos