search

ThreeMammals / Ocelot

.NET API Gateway
https://www.nuget.org/packages/Ocelot
MIT License
8.38k stars 1.64k forks source link

#1708 Generic polling mechanism for service discovery #1710

Closed ggnaegi closed 7 months ago

ggnaegi commented 1 year ago

Closes #1708

Providers included: Consul, Eureka, etc?...

Proposed Changes

ggnaegi commented 1 year ago

@raman-m This is the PR we talked about, including a generic polling manager.

raman-m commented 1 year ago

Thanks for the PR! 👍

You have to update your feature branch from develop one! ... because of this commit e5f31ef030a4004579da8c54d663721ffb58fa22 after merging #1711 And, unfortunately you have to resolve merge conflicts! 😢

Please be very accurate in such kind of PRs with aggressive refactoring! Watch for interconnections, double check facts, and of course care about all tests. I will return to your PR back after current release these days... Thanks you! Having a good start of the week!... 🍂 (autumn mood)

ggnaegi commented 1 year ago

Thanks for the PR! 👍

You have to update your feature branch from develop one! ... because of this commit e5f31ef after merging #1711 And, unfortunately you have to resolve merge conflicts! 😢

@raman-m merge is done, but I will review the code once or twice and let you know.

raman-m commented 1 year ago

Don't you like your idea described in #1708 ?

You're the author, you can close it anytime. But idea was right. Unfortunately we haven't discussed this PR...

ggnaegi commented 1 year ago

@raman-m a new provider project is needed for ServiceFabric... Can I create a new project in the solution could you add me as assignee?

raman-m commented 1 year ago

@ggnaegi commented on Oct 15

You're assigned! 😉

raman-m commented 11 months ago

Rebasing feature branch is required!

raman-m commented 11 months ago

Wow! I remember this PR... 😄 It is not planned for any milestones.. What is the approximate readiness now?

ggnaegi commented 11 months ago

Wow! I remember this PR... 😄 It is not planned for any milestones.. What is the approximate readiness now?

Still a bit of work, but we could foresee it for the december release.

raman-m commented 8 months ago

@ggnaegi Multiple conflicts! 👇 Please resolve them!

Still a bit of work, but we could foresee it for the december release.

Well... Dec'23 release is closed. Do you want to include it to Jan'24 release on Feb 23 or leave it in Annual'23 one? Will 1 week be sufficient to finish the PR?

ggnaegi commented 8 months ago

@raman-m this should be part of Annual release, first address the consul long polling issue.

raman-m commented 7 months ago

@ggnaegi I can't have a quick resolving of all these merge conflicts. I don't know the logic, there are a lot of conflicts. So, I can't rebase the branch onto target!

Could you resolve merge conflicts please?

Don't forget to make backup copied branch or copy whole solution to a separate folder.

ggnaegi commented 7 months ago

raman-m commented on Apr 7

@raman-m I think I will need to rewrite it since I'm preparing a PR with the long polling for consul...

raman-m commented 7 months ago

So, is this PR dependent on a PR which will be created soon? Why keep this open? Mark draft?

Do you need to re-write this PR keeping it open, or will you close this PR and open new one after dependency removed?

ggnaegi commented 7 months ago

So, is this PR dependent on a PR which will be created soon? Why keep this open? Mark draft?

Do you need to re-write this PR keeping it open, or will you close this PR and open new one after dependency removed?

I think I will close this PR, open a new one with the long polling for the consul provider... And later reopen a PR with a generalized mechanism. Keep it as a draft for now.