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

Upgrade eureka to be compatible with jakartaee #1465

Merged
merged 15 commits into from
Sep 30, 2022

Conversation

spencergibb
Copy link
Contributor

@spencergibb spencergibb commented Aug 19, 2022

  • Remove jersey 1
  • Upgrade to jakarta servlet 5
  • Upgrade to jakarta inject 2
  • Upgrade jersey 2 to jersey 3.0.5
  • Remove guice Upgrade guice to com.guicedee.services:guice:1.2.2.1
  • Upgrades test libs: mockito, jetty, etc...
  • Build with java 11, target java 8 (jetty 11 requires java 11)
  • Move eureka-core integration tests (anything that referenced jersey) to new eureka-tests module
  • Upgrade CI to build with java 11
  • Move eureka-client integration tests to new eureka-tests module
  • rename jersey2 classes, modules to jersey3
  • Upgrade version to 2.0.0-SNAPSHOT
  • Deal with eureka-server-governator (governator is not jakartaee compatible
  • Deal with tests that use governator LifecycleInjector
  • Add functionality lost (cross region headers?) by removing apache HTTP client
  • Fix Ignored Tests in DiscoveryClientCloseJerseyThreadTest
  • Fix Ignored Tests in DiscoveryClientRegisterUpdateTest
  • Fix Ignored Tests in DiscoveryClientStatsTest
  • Fix Ignored Tests in EurekaClientServerRestIntegrationTest
  • Fix Ignored Tests in EurekaHttpClientCompatibilityTestSuite
  • Fix Ignored Tests in TimeConsumingInstanceRegistryTest
  • Verify all remaining FIXME 2.0 todos are dealt with

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()) {
Copy link
Contributor Author

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.

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?

@spencergibb spencergibb changed the base branch from master to 2.x September 21, 2022 19:38
@spencergibb spencergibb changed the base branch from 2.x to master September 21, 2022 19:38
@spencergibb spencergibb changed the base branch from master to 2.x September 26, 2022 20:34
@spencergibb spencergibb marked this pull request as ready for review September 29, 2022 17:19
@spencergibb spencergibb mentioned this pull request Sep 29, 2022
8 tasks
Copy link

@howardyuan howardyuan left a 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.

@howardyuan howardyuan merged commit bf63794 into Netflix:2.x Sep 30, 2022
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.

2 participants