[Draft] Upgrade JDK target to 17 #735

Andyz26 · 2024-12-20T22:32:03Z

Context

Upgrade project to target JDK17.

Checklist

./gradlew build compiles code correctly
Added new tests where applicable
./gradlew test passes all tests
Extended README or added javadocs where applicable

Andyz26 · 2024-12-20T22:34:10Z

@crioux-stripe @kmg-stripe @garryliu-stripe How do you feel about the moving towards jdk17 for the OSS components? Any thing internal you need to keep on jdk8? This is still early draft and I have some internal validation to do but I would like to see how you folks feel about this early.

github-actions · 2024-12-20T22:39:29Z

Test Results

142 files ±0 142 suites ±0 8m 1s ⏱️ -27s
619 tests ±0 608 ✅ ±0 11 💤 +1 0 ❌ - 1
619 runs - 1 608 ✅ - 1 11 💤 +1 0 ❌ - 1

Results for commit 1909c32. ± Comparison against base commit 35a0826.

This pull request skips 1 test.

io.mantisrx.publish.internal.discovery.MantisJobDiscoveryTest ‑ testJobDiscoveryFetchFailureHandlingAfterSuccess()

♻️ This comment has been updated with latest results.

github-actions · 2024-12-20T22:39:55Z

Uploaded Artifacts

To use these artifacts in your Gradle project, paste the following lines in your build.gradle.

resolutionStrategy {
    force "io.mantisrx:mantis-discovery-proto:0.1.0-20241220.224239-563"
    force "io.mantisrx:mantis-common-serde:0.1.0-20241220.224239-563"
    force "io.mantisrx:mantis-common:0.1.0-20241220.224239-563"
    force "io.mantisrx:mantis-client:0.1.0-20241220.224239-564"
    force "io.mantisrx:mantis-network:0.1.0-20241220.224239-563"
    force "io.mantisrx:mantis-runtime:0.1.0-20241220.224239-564"
    force "io.mantisrx:mantis-runtime-loader:0.1.0-20241220.224239-564"
    force "io.mantisrx:mantis-remote-observable:0.1.0-20241220.224239-564"
    force "io.mantisrx:mantis-shaded:0.1.0-20241220.224239-562"
    force "io.mantisrx:mantis-testcontainers:0.1.0-20241220.224239-233"
    force "io.mantisrx:mantis-runtime-executor:0.1.0-20241220.224239-99"
    force "io.mantisrx:mantis-rxcontrol:0.1.0-20241220.224239-37"
    force "io.mantisrx:mantis-connector-publish:0.1.0-20241220.224239-563"
    force "io.mantisrx:mantis-connector-job-source:0.1.0-20241220.224239-15"
    force "io.mantisrx:mantis-connector-kafka:0.1.0-20241220.224239-564"
    force "io.mantisrx:mantis-control-plane-client:0.1.0-20241220.224239-563"
    force "io.mantisrx:mantis-connector-iceberg:0.1.0-20241220.224239-562"
    force "io.mantisrx:mantis-control-plane-core:0.1.0-20241220.224239-557"
    force "io.mantisrx:mantis-control-plane-dynamodb:0.1.0-20241220.224239-24"
    force "io.mantisrx:mantis-control-plane-server:0.1.0-20241220.224239-557"
    force "io.mantisrx:mantis-examples-groupby-sample:0.1.0-20241220.224239-557"
    force "io.mantisrx:mantis-examples-core:0.1.0-20241220.224239-557"
    force "io.mantisrx:mantis-examples-jobconnector-sample:0.1.0-20241220.224239-557"
    force "io.mantisrx:mantis-examples-twitter-sample:0.1.0-20241220.224239-557"
    force "io.mantisrx:mantis-examples-mantis-publish-sample:0.1.0-20241220.224239-557"
    force "io.mantisrx:mantis-examples-synthetic-sourcejob:0.1.0-20241220.224239-557"
    force "io.mantisrx:mantis-examples-wordcount:0.1.0-20241220.224239-556"
    force "io.mantisrx:mantis-publish-core:0.1.0-20241220.224239-556"
    force "io.mantisrx:mantis-examples-sine-function:0.1.0-20241220.224239-556"
    force "io.mantisrx:mantis-publish-netty-guice:0.1.0-20241220.224239-557"
    force "io.mantisrx:mantis-publish-netty:0.1.0-20241220.224239-556"
    force "io.mantisrx:mantis-server-worker-client:0.1.0-20241220.224239-557"
    force "io.mantisrx:mantis-source-job-publish:0.1.0-20241220.224239-557"
    force "io.mantisrx:mantis-server-agent:0.1.0-20241220.224239-556"
    force "io.mantisrx:mantis-source-job-kafka:0.1.0-20241220.224239-557"
}

crioux-stripe · 2024-12-23T14:00:54Z

@Andyz26 I think we'd be pretty keen to move to JDK 17! We're running 11 on the Mantis hosts right now but most of our Java monorepo is 17+. I don't think anything is preventing us from running 17, and we may have prototyped with it IIRC.

codyrioux · 2025-03-11T21:05:34Z

Writing in to confirm that this compiled and passed all tests on:

Apple Macbook Pro M1 Max, Zulu JDK17.
Windows Subsystem for Linux with OpenJDK17.

cody@DESKTOP-VBJ42TN:~/src/mantis$ java --version
openjdk 17.0.14 2025-01-21
OpenJDK Runtime Environment (build 17.0.14+7-Ubuntu-124.04)
OpenJDK 64-Bit Server VM (build 17.0.14+7-Ubuntu-124.04, mixed mode, sharing)
cody@DESKTOP-VBJ42TN:~/src/mantis$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 24.04.2 LTS
Release:        24.04
Codename:       noble

We've cut a release candidate with version 4.0.0-rc.1 with hopes that @andresgalindo-stripe can test it in Stripe's mantisdev environment.

codyrioux

A few very small comments, nothing blocking.

@andresgalindo-stripe Not to put you in the hot seat but I think I'd like to hear a thumbs up from you before we hit the merge button.

codyrioux · 2025-03-12T01:37:45Z

mantis-connectors/mantis-connector-kafka/build.gradle

-    testImplementation libraries.mockitoAll
-    testImplementation "com.github.tomakehurst:wiremock-jre8:2.21.0"
+    testImplementation libraries.mockitoCore
+    testImplementation libraries.wiremockjre8


Can probably just use org.wiremock:wiremock now.

codyrioux · 2025-03-12T01:39:13Z

...mantis-control-plane-server/src/test/java/io/mantisrx/master/api/akka/route/JacksonTest.java

+    private static final ObjectMapper objectMapper = new ObjectMapper()
+        .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)
+        .configure(SerializationFeature.FAIL_ON_EMPTY_BEANS, false)
+        .registerModule(new Jdk8Module());


As far as I know this introduced features in Jackson like not requiring @JsonProperty in Java 8. As best I can tell these are now all handled via Jackson. We can likely skip the module, but probably no harm in including it.

iirc these were added due to some datetime format test failure.

codyrioux · 2025-03-12T01:40:16Z

mantis-publish/mantis-publish-core/build.gradle

    implementation libraries.slf4jApi

+    testImplementation libraries.junit4
+    testImplementation libraries.wiremockjre8


Probably can just use regular old wiremock.

codyrioux · 2025-03-12T01:40:48Z

...ublish-core/src/test/java/io/mantisrx/publish/internal/discovery/MantisJobDiscoveryTest.java

@@ -121,8 +121,8 @@ public void testJobDiscoveryFetch() throws IOException {
        assertEquals(jobSchedulingInfo.getWorkerAssignments().get(1).getHosts().get(1).getHost(), currentJobWorkers.get().getIngestStageWorkers().getWorkers().get(0).getHost());
    }

+    @Disabled("another flaky test")


Do we know what makes this test flaky? Is it inherent to the code or the test itself?

Actually I'm withdrawing this comment from this review. This PR didn't introduce this test, and is just renaming the annotation from @Ignore to @Disabled.

Andyz26 added 3 commits December 20, 2024 14:17

jdk17 on gradle build

6fcbb01

deps

d7d8c42

move target jdk

7625c00

Andyz26 requested review from calvin681, sundargates, hmitnflx and fdc-ntflx as code owners December 20, 2024 22:32

Andyz26 temporarily deployed to Integrate Pull Request December 20, 2024 22:32 — with GitHub Actions Inactive

update github actions

1909c32

Andyz26 temporarily deployed to Integrate Pull Request December 20, 2024 22:42 — with GitHub Actions Inactive

codyrioux approved these changes Mar 12, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Draft] Upgrade JDK target to 17 #735

[Draft] Upgrade JDK target to 17 #735

Andyz26 commented Dec 20, 2024

Andyz26 commented Dec 20, 2024 •

edited

Loading

github-actions bot commented Dec 20, 2024 •

edited

Loading

github-actions bot commented Dec 20, 2024 •

edited

Loading

crioux-stripe commented Dec 23, 2024

codyrioux commented Mar 11, 2025

codyrioux left a comment

codyrioux Mar 12, 2025

codyrioux Mar 12, 2025

Andyz26 Mar 12, 2025

codyrioux Mar 12, 2025

codyrioux Mar 12, 2025

codyrioux Mar 12, 2025

[Draft] Upgrade JDK target to 17 #735

Are you sure you want to change the base?

[Draft] Upgrade JDK target to 17 #735

Conversation

Andyz26 commented Dec 20, 2024

Context

Checklist

Andyz26 commented Dec 20, 2024 • edited Loading

github-actions bot commented Dec 20, 2024 • edited Loading

Test Results

github-actions bot commented Dec 20, 2024 • edited Loading

Uploaded Artifacts

crioux-stripe commented Dec 23, 2024

codyrioux commented Mar 11, 2025

codyrioux left a comment

Choose a reason for hiding this comment

codyrioux Mar 12, 2025

Choose a reason for hiding this comment

codyrioux Mar 12, 2025

Choose a reason for hiding this comment

Andyz26 Mar 12, 2025

Choose a reason for hiding this comment

codyrioux Mar 12, 2025

Choose a reason for hiding this comment

codyrioux Mar 12, 2025

Choose a reason for hiding this comment

codyrioux Mar 12, 2025

Choose a reason for hiding this comment

Andyz26 commented Dec 20, 2024 •

edited

Loading

github-actions bot commented Dec 20, 2024 •

edited

Loading

github-actions bot commented Dec 20, 2024 •

edited

Loading