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

Refactor eureka related classes to separate module #718

Conversation

asafalima
Copy link

@asafalima asafalima commented Jan 31, 2020

We wanted to remove the need for eureka from zuul-core for few reasons:

  • We don't use it in our organisation, and like us there are others which use different service discovery solution.
  • Eureka uses Jersey 1.x and JAX-RS 1.x which conflicts with our rest-sever which is based on Jersey 2.x
  • Avoid unnecessary dependencies

To accomplish this goal I made the following changes:

  • Created a new module zuul-eureka and moved any eureka related file to it.
  • Added ServerStatusManager interface in core module and EurekaServerStatusManager implementation in the eureka module.
  • Added ServerIpAddrExtractor interface in core module and DiscoveryServerIpAddrExtractor implementation in the eureka module which extracts the IP address from DiscoveryEnabledServer.
  • Refactored out the out-of-service clients shutdown logic from ClientConnectionsShutdown to a separate class OutOfServiceConnectionsShutdown which resided in the eureka module.
  • Remove DiscoveryEnabledServer related logic from RequestAttempt's constructor and moved it to the newRequestAttempt method in a new implementation of NettyOrigin: DiscoveryEnabledNettyOrigin which extends BasicNettyOrigin.

@asafalima asafalima force-pushed the refactor-eureka-related-classes-to-separate-module branch from 3c15f57 to 4d39106 Compare January 31, 2020 01:05
@asafalima
Copy link
Author

@carl-mastrangelo @artgon I would really appreciate your input about this change.

@asafalima asafalima force-pushed the refactor-eureka-related-classes-to-separate-module branch from 4d39106 to 16367e0 Compare March 11, 2020 21:26
@asafalima
Copy link
Author

@carl-mastrangelo @artgon I need your help with pushing it forward

@artgon
Copy link
Contributor

artgon commented Mar 11, 2020

Hi @asafalima, this is a reasonable goal but this would break a lot of stuff for us internally. We are too coupled to Eureka and Ribbon in our internal Zuul.

We do have some future plans to address these coupling problems. Likely in the next major release of Zuul.

@artgon artgon closed this Mar 11, 2020
@mattnelson
Copy link

  • Eureka uses Jersey 1.x and JAX-RS 1.x which conflicts with our rest-sever which is based on Jersey 2.x

@asafalima Eureka has jersey2 implementations available.

https://github.com/Netflix/eureka/tree/master/eureka-client-jersey2
https://github.com/Netflix/eureka/tree/master/eureka-core-jersey2

@asafalima
Copy link
Author

Hi @artgon,

I saw @carl-mastrangelo indeed noted this as one of the goals for V3 in #771
Maybe we can make some changes to the work that was already done here to make it fit?

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

Successfully merging this pull request may close these issues.

3 participants