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

Closed
wants to merge 3 commits into from
Closed

Allow load balancer uri in uri attribute #518

wants to merge 3 commits into 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 #489
@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)

@antechrestos antechrestos closed this by deleting the head repository Jul 8, 2024
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