Ocelot
Ocelot copied to clipboard
ThreeMammals
Fix the problem while doing first call for service to gateway using p…
Fixes / New Feature # Fix for pollconsul provider for the first request to any service while API gateway uses pollconsul provider.
Proposed Changes
- Fix the problem of first request before timer will start fetching services from consul
I could have a look...
Hi Przemyslaw! Could you resolve merge conflicts first, please?
Don't forget to update your feature branch by latest top-commits from base:develop branch!
Przemyslaw! There are a lot of merge conflicts! I guess current feature branch is outdated. It is better to backup your changes to some folder. And re-fork the Ocelot repo. SO, you will have top commits from head repo. And create new feature branch from develop. Apply your code changes. Make new PR please!
Or go to my forked Ocelot repo. And it says: "This branch is up to date with ThreeMammals/Ocelot:develop." I can add you as Contributor. You will be able to create feature branches.
Do you have an intention to have some pair programming?
@przemyslawandruszewski Przemyslaw, I thought you needed my help to resolve merge conflicts. 😉 The feature branch has been rebased onto ThreeMammals:develop successfully! I've additionally linked to the issue from your past comment.
Good luck during code review!
@chaitanyaphanikumar @lufeiyuan83 @ggnaegi Please, join to code review session!
@raman-m This should be addressed in https://github.com/ThreeMammals/Ocelot/pull/1670, no?
@ggnaegi commented on Sep 18
Yes, it is addressed, but this PR has extra enhancement of injecting IMemoryCache object into PollConsul constructor! 😉
@ggnaegi commented on Sep 18
Yes, it is addressed, but this PR has extra enhancement of injecting IMemoryCache object into
PollConsulconstructor! 😉
Like @RaynaldM I think, we are mixing concerns here...
@ggnaegi By other words, if we remove caching logic here, this PR become duplicate of yours, right?
Yes, it seems caching inside of provider is bad idea. The provider should communicate to Consul service only. I guess, the author tried to improve performance of the provider, after facing with errors, and this PR is just a try to fix stability of the provider. But this has been addressed in your PR already.
@przemyslawandruszewski In future we can inject caching interface, but it should not be memory cache, otherwise Ocelot will duplicate all collections from Consul service... I understand, you've tried to improve the performance, but the root cause is Sorry, cannot accept it nowadays. Cache must be more abstract. And we cannot add this caching forcibly, so configuration property is required. If you have an intention to contribute, please return back with reopen or message.
Again, Thanks you for the PR!