-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Upgrade eureka to be compatible with jakartaee #1465
Conversation
Replaces javax.{inject,annotation} with jakarta.{inject,annotation}. Requires java 11 to build (jetty 11 in tests), but continues to target java 8. Replaces google guice with guicedee guice that supports jakarta.inject.
Uses EurekaHttpClient as the interface used in DI
Removes unneeded JerseyReplicationClientTest.java
logger.info("Getting instance registry info from the eureka server : {} , delta : {}", this.remoteRegionURL, delta); | ||
|
||
if (shouldUseExperimentalTransport()) { | ||
// if (shouldUseExperimentalTransport()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionally, this is probably the biggest change I've made, relying on the EurekaHttpClient
interface rather that trying to upgrade the usage of the internal jersey1 apache HTTP client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like the internal discovery client takes care of cross region identity header, but EurekaHttpClient doesn't?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume all //FIXME 2.0 places will be eventually fixed before the official release. Otherwise, LGTM.
Upgrade guice to com.guicedee.services:guice:1.2.2.1LifecycleInjector
FIXME 2.0
todos are dealt with