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

Allow load balancer uri in uri attribute #518

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

antechrestos
Copy link

@antechrestos antechrestos commented Mar 30, 2021

Description

Fixes #489 : This change is the one requested by late issue . The aim is to handle url such as lb://some-service (or `lbs://some-service). When such url is specified, we ensure load balance client is provided and we keep it instead of using delegate one).

Code implementation

I willingly kept 3 commit to better describe the implementation

  • improve code specification of name, url and path handle by adding tests
  • refactoring of code, particularly by forcing reentrance of code (url was modified by side effect)
  • implementing the feature

Ben Einaudi added 3 commits March 30, 2021 18:05
By specifing 'lb://' protocol, feign client factory will use load
balance client instead of using delegate client

Closes spring-cloud#489
@antechrestos antechrestos force-pushed the features/allow-load-balancer-uri-in-uri-attribute branch from 1595fff to 5661a35 Compare March 30, 2021 17:02
@codecov
Copy link

codecov bot commented Mar 30, 2021

Codecov Report

Merging #518 (5661a35) into master (bcea218) will increase coverage by 0.55%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #518      +/-   ##
============================================
+ Coverage     77.85%   78.41%   +0.55%     
- Complexity      489      499      +10     
============================================
  Files            60       60              
  Lines          1820     1816       -4     
  Branches        267      266       -1     
============================================
+ Hits           1417     1424       +7     
+ Misses          256      249       -7     
+ Partials        147      143       -4     
Impacted Files Coverage Δ Complexity Δ
...mework/cloud/openfeign/FeignClientFactoryBean.java 76.49% <100.00%> (+4.55%) 74.00 <12.00> (+10.00)

@Sam-Kruglov
Copy link
Contributor

Are you sure it's related to 499? Doesn't seem like it is to me...

@antechrestos
Copy link
Author

@Sam-Kruglov Yeah sorry. Thank you for the warning. 🙏

@OlgaMaciaszek
Copy link
Collaborator

See #489 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support lb:// url in FeingClient annotation
4 participants