Skip to content

GH-475: Optional Feign clients eager initialization mechanism #512

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

Closed
wants to merge 2 commits into from
Closed

GH-475: Optional Feign clients eager initialization mechanism #512

wants to merge 2 commits into from

Conversation

Periecle
Copy link

@Periecle Periecle commented Mar 14, 2021

Pull Request majorly influenced by similar Ribbon feature, therefore API and configuration are close to each other for usability.
Also updated documentation explaining configuration and implementation details.
Fixes #475

@codecov
Copy link

codecov bot commented Mar 14, 2021

Codecov Report

Merging #512 (cc04f56) into master (61aee7c) will increase coverage by 0.16%.
The diff coverage is 94.44%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #512      +/-   ##
============================================
+ Coverage     77.85%   78.01%   +0.16%     
- Complexity      489      496       +7     
============================================
  Files            60       62       +2     
  Lines          1820     1838      +18     
  Branches        267      267              
============================================
+ Hits           1417     1434      +17     
- Misses          256      257       +1     
  Partials        147      147              
Impacted Files Coverage Δ Complexity Δ
...work/cloud/openfeign/FeignEagerLoadProperties.java 88.88% <88.88%> (ø) 4.00 <4.00> (?)
.../openfeign/FeignApplicationContextInitializer.java 100.00% <100.00%> (ø) 2.00 <2.00> (?)
...mework/cloud/openfeign/FeignAutoConfiguration.java 82.81% <100.00%> (+0.55%) 4.00 <0.00> (ø)
.../springframework/cloud/openfeign/FeignContext.java 100.00% <100.00%> (ø) 4.00 <1.00> (+1.00)

@Periecle Periecle changed the title GH-475: Optional Feign clients eager initializing mechanism GH-475: Optional Feign clients eager initialization mechanism Mar 14, 2021
@OlgaMaciaszek OlgaMaciaszek self-assigned this Mar 16, 2021
Copy link
Collaborator

@OlgaMaciaszek OlgaMaciaszek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Periecle thanks for submitting this. Have added some comments to address, however, have also run some tests on the sample provided in the issue (#475) and this does not really seem to solve the issue, so probably will require further investigation.

@Periecle
Copy link
Author

Periecle commented Mar 16, 2021

@Periecle thanks for submitting this. Have added some comments to address, however, have also run some tests on the sample provided in the issue (#475) and this does not really seem to solve the issue, so probably will require further investigation.

@OlgaMaciaszek , author of original issue was struggling with initialising feign clients in async way from ForkJoinPool, and you proposed an alternative - implement something similar to ribbon.eager-load mechanism. I think my PR won't fix sample from original issue, but propose options to provide such functionality directly without need for such workarounds with supplying fake request to FJP or any other thread pool.
UPD: On second thought, it seems that cloud-openfeign need option for initialising Ribbon LB on ApplicationReadyEvent optionally on feign.ribbon.warmup, . Actually, I am already working on this feature.

@Periecle Periecle requested a review from OlgaMaciaszek March 16, 2021 20:12
@Periecle
Copy link
Author

@OlgaMaciaszek, hi. Can you update me with current status of review?

@OlgaMaciaszek
Copy link
Collaborator

hi. Can you update me with current status of review?

@Periecle , I understood you were still working on this bit for the PR:

On second thought, it seems that cloud-openfeign need option for initialising Ribbon LB on ApplicationReadyEvent optionally on feign.ribbon.warmup, . Actually, I am already working on this feature.

Or will you be submitting it in another PR?

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

Successfully merging this pull request may close these issues.

Feign client lazy initialization fails when using CompletableFuture.supplyAsync() to call the Feign client initially
3 participants