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.

Sign up for GitHub

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

Jump to bottom

#1708 Generic polling mechanism for service discovery #1710

Closed
wants to merge 10 commits into from
Closed

#1708 Generic polling mechanism for service discovery #1710

wants to merge 10 commits into from

Conversation

ggnaegi
Copy link
Member

@ggnaegi ggnaegi commented Sep 29, 2023 •
edited by raman-m
Loading

Closes #1708

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

Proposed Changes

  • Introduce PollingServicesManager<T, TU> for generic cluster management
  • Introduce ServicePollingHandler<T> for generic destinations management and polling
  • Use the standardized classes in PollConsul, PollKube
  • Introduce PollEureka

@ggnaegi
Copy link
Member Author

ggnaegi commented Sep 30, 2023

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

@ggnaegi ggnaegi changed the title Proposing a generalized polling mechanism for discovery services, lik… Proposing a generic polling mechanism for discovery services, lik… Sep 30, 2023
@raman-m
Copy link
Member

raman-m commented Oct 2, 2023

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! 😢

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)

@raman-m raman-m added the conflicts Feature branch has merge conflicts label Oct 2, 2023
@ggnaegi
Copy link
Member Author

ggnaegi commented Oct 2, 2023

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.

@ggnaegi ggnaegi closed this Oct 9, 2023
@ggnaegi ggnaegi removed their assignment Oct 9, 2023
@raman-m
Copy link
Member

raman-m commented Oct 9, 2023 •
edited
Loading

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 ggnaegi reopened this Oct 13, 2023
@ggnaegi
Copy link
Member Author

ggnaegi commented Oct 15, 2023 •
edited
Loading

@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 raman-m changed the title Proposing a generic polling mechanism for discovery services, lik… #1708 Generic polling mechanism for service discovery Oct 16, 2023
@raman-m raman-m removed the conflicts Feature branch has merge conflicts label Oct 16, 2023
@raman-m raman-m added feature A new feature proposal Proposal for a new functionality in Ocelot Service Discovery Ocelot feature: Service Discovery labels Oct 16, 2023
@raman-m
Copy link
Member

raman-m commented Oct 16, 2023

@ggnaegi commented on Oct 15

You're assigned! 😉

@raman-m
Copy link
Member

raman-m commented Nov 22, 2023

Rebasing feature branch is required!

@raman-m
Copy link
Member

raman-m commented Nov 22, 2023

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

@ggnaegi
Copy link
Member Author

ggnaegi commented Nov 22, 2023

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 raman-m added this to the December'23 milestone Nov 25, 2023
@raman-m raman-m added the 2023 Annual 2023 release label Nov 25, 2023
@raman-m raman-m added the conflicts Feature branch has merge conflicts label Feb 17, 2024
@raman-m
Copy link
Member

raman-m commented Feb 17, 2024 •
edited
Loading

@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
Copy link
Member Author

ggnaegi commented Feb 17, 2024

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

@raman-m raman-m changed the base branch from develop to release/24.0 March 5, 2024 09:46
@raman-m
Copy link
Member

raman-m commented Apr 7, 2024

@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
Copy link
Member Author

ggnaegi commented Apr 7, 2024 •
edited by raman-m
Loading

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
Copy link
Member

raman-m commented Apr 7, 2024

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?

@raman-m raman-m marked this pull request as draft April 7, 2024 14:55
@ggnaegi
Copy link
Member Author

ggnaegi commented Apr 7, 2024

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.

@ggnaegi ggnaegi closed this Apr 8, 2024
@raman-m raman-m added Spring'24 Spring 2024 release and removed 2023 Annual 2023 release labels Oct 4, 2024
@raman-m raman-m modified the milestones: Annual 2023, Spring'24 Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@ggnaegi ggnaegi

Labels
conflicts Feature branch has merge conflicts feature A new feature proposal Proposal for a new functionality in Ocelot Service Discovery Ocelot feature: Service Discovery Spring'24 Spring 2024 release
Projects
None yet
Milestone
Spring'24
Development

Successfully merging this pull request may close these issues.

Generic polling mechanism for discovery services (Consul, Eureka...)
2 participants
@ggnaegi @raman-m