Support Java 8 for opensearch-rest-client #3181

dlvenable · 2022-05-04T14:58:55Z

Description

This PR sets the source and target compatibility to Java 8 for the opensearch-rest-client. Additionally, it sets the client-test project to use Java 8 since opensearch-rest-client depends on it.

This should ensure that only Java 8 language features compile for these projects. However, it cannot verify that only Java 8 APIs are in use. I believe OpenSearch would need to use Gradle toolkits and run on a machine with both Java 11 and Java 8 for this.

Issues Resolved

This addresses:
opensearch-project/opensearch-java#156

Verification

Download the 2.0.0-rc1 opensearch-rest-client:

wget https://repo1.maven.org/maven2/org/opensearch/client/opensearch-rest-client/2.0.0-rc1/opensearch-rest-client-2.0.0-rc1.module

Examine the Gradle module file. It requires Java 11.

grep jvm opensearch-rest-client-2.0.0-rc1.module
        "org.gradle.jvm.version": 11,
        "org.gradle.jvm.version": 11,

Build OpenSearch locally with the change.

./gradlew clean assemble
./gradlew publishToMavenLocal

Verify Java 8 in the Gradle module file

grep jvm ~/.m2/repository/org/opensearch/client/opensearch-rest-client/3.0.0-SNAPSHOT/opensearch-rest-client-3.0.0-SNAPSHOT.module
        "org.gradle.jvm.version": 8,
        "org.gradle.jvm.version": 8,

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

…t to Java 8. This project uses a client test library which is now also supporting Java 8. Signed-off-by: David Venable <[email protected]>

opensearch-ci-bot · 2022-05-04T15:31:40Z

✅ Gradle Check success ef81f58269e74cc18d7bb465ebf467e49252284f
Log 4993

Reports 4993

opensearch-ci-bot · 2022-05-04T15:39:30Z

✅ Gradle Check success ad6bd4a
Log 4994

Reports 4994

reta · 2022-05-04T16:23:55Z

@dlvenable I don't think we should go forward with this change:

moving baseline to JDK-11 is intentional (and aligned with projects Opensearch depends on)
we recommend to use highlevel-rest-client and it requires JDK-11
downgrading rest-client to 1.8 won't help opensearch-java since it depends on test framework [1] and will require JDK-11

[1] https://github.com/opensearch-project/opensearch-java/blob/main/java-client/build.gradle.kts#L132

dblock · 2022-05-04T19:11:40Z

I agree with @reta.

But if we decided to do this, for starters, we would need to also re-add Java 8 CI but only run tests for this part of the project on JDK8. What's the pull for that?

dblock

Let's leave this open for now until we have a path forward for clients that run on JDK8. Thanks for your help @dlvenable.

Signed-off-by: David Venable <[email protected]>

opensearch-ci-bot · 2022-05-06T16:45:36Z

❌ Gradle Check failure 5283072
Log 5088

Reports 5088

dblock

I am not against this.

Try adding a JDK8 CI part of this? Without it we have no hope of ever preserving future compatibility.

dblock · 2022-05-10T18:52:08Z

client/rest/build.gradle

@@ -57,7 +68,7 @@ dependencies {
 }

 tasks.withType(CheckForbiddenApis).configureEach {
-  //client does not depend on server, so only jdk and http signatures should be checked
+  //client does not depend on shamcreerver, so only jdk and http signatures should be checked


bulk replace at work? :)

wbeckler · 2023-02-09T19:27:12Z

Any thoughts on the next steps on this? If someone's client has to be Java 8, what's the recommendation for them? Here's an example of someone who is stuck due to the java 11 requirement for opensearch-java: https://forum.opensearch.org/t/opensearch-1-3-x-gradle-build-problem-w-java-8/12400

reta · 2023-02-10T00:50:51Z

Any thoughts on the next steps on this? If someone's client has to be Java 8, what's the recommendation for them? Here's an example of someone who is stuck due to the java 11 requirement for opensearch-java: https://forum.opensearch.org/t/opensearch-1-3-x-gradle-build-problem-w-java-8/12400

@wbeckler I think, what we could here is:

rest-client goes back to JDK-8 baseline (@dblock we don't need to have JDK-8 CI for it, setting source / target level to 1.8 or release should be enough)
opensearch-java goes back to JDK-8 baseline, but we would need to build it with JDK-11 (or above) since it uses test framework from core

@dblock @dlvenable what do you think folks?

dlvenable · 2023-02-10T03:09:00Z

rest-client goes back to JDK-8 baseline (@dblock we don't need to have JDK-8 CI for it, setting source / target level to 1.8 or release should be enough)

Is there a need to move this back to JDK 8? It seems work in opensearch-java can provide users with an option of using JDK 8. Of course, this does mean that users would need to use opensearch-java instead of rest-client.

opensearch-java goes back to JDK-8 baseline, but we would need to build it with JDK-11 (or above) since it uses test framework from core

If users are still blocked on JDK 8, then I think we should do this. The recent changes will allow opensearch-java to decouple from OpenSearch core and have its own JDK versions based on user needs.

reta · 2023-02-10T12:50:47Z

Is there a need to move this back to JDK 8? It seems work in opensearch-java can provide users with an option of using JDK 8.

That's the question :D The consensus is that opensearch-java should be the only official client to use (but there is long road ahead). We could not downgrade it to JDK-8 without rest-client (in a proper way).

The recent changes will allow opensearch-java to decouple from OpenSearch core and have its own JDK versions based on user needs.

Yes and no, we still have to support rest-client based transport, and if we want to do that in a way to not require users to use JDK-11 or above, we have to be on JDK-8 all way through.

andrross · 2023-02-10T17:21:27Z

The recent changes will allow opensearch-java to decouple from OpenSearch core and have its own JDK versions based on user needs.

Yes and no, we still have to support rest-client based transport, and if we want to do that in a way to not require users to use JDK-11 or above, we have to be on JDK-8 all way through.

@reta Is there anyway to accelerate this decoupling? I know it's gross but could we copy the rest-client implementation into opensearch-java? Support for a JDK-8 client seems like a reasonable request but I would hate to burden the entire server implementation with JDK-8 support due to this coupling.

reta · 2023-02-10T17:40:57Z

@reta Is there anyway to accelerate this decoupling? I know it's gross but could we copy the rest-client implementation into opensearch-java?

Hm ... let see, if we copy, we have to preserve the same packaging (so people won't see the difference) - this part is fine. But if there is an old rest-client dependency on the classpath (transitive or not) - we are in trouble, more than one class under the same package on classpath, that would not be fun.

Support for a JDK-8 client seems like a reasonable request but I would hate to burden the entire server implementation with JDK-8 support due to this coupling.

Oh, rest-client would be one off exception - to stay on JDK-8 bytecode, everything else moves forward.

andrross · 2023-02-10T18:30:19Z

@reta Ah, got it, I think I'm up to speed now :)

opensearch-java goes back to JDK-8 baseline, but we would need to build it with JDK-11 (or above) since it uses test framework from core

If we do this, and then let's say opensearch-java takes a dependency on something newly introduced in JDK-11, will the build and/or tests fail even if we're running with JDK-11? Because it would definitely fail at runtime with JDK-8.

reta · 2023-02-10T18:34:41Z

Because it would definitely fail at runtime with JDK-8.

Yes, we need to make sure that opensearch-java does not use features which are not supported by JDK-8, but that we could do with source/target or release setting (so the build will fail).

dlvenable · 2023-02-10T19:27:57Z

Hm ... let see, if we copy, we have to preserve the same packaging (so people won't see the difference) - this part is fine. But if there is an old rest-client dependency on the classpath (transitive or not) - we are in trouble, more than one class under the same package on classpath, that would not be fun.

This can be considered a breaking change. I believe the last release of opensearch-java was 2.2.0 so there is still a 3.0.0 available. We can give clear guidance on how to remove the transitive dependency on the old rest-client as part of the instructions.

Alternatively, we could use a different Java package name from the original. This would require that customers update all of their package names in their code. But, it would probably be safer.

This is a hard change to be making, but I believe that this decoupling will be great for users!

andrross · 2023-02-10T19:48:56Z

This can be considered a breaking change.

Is it feasible to remove the OpenSearch core dependency and RestClientTransport all together in 3.0 and instruct users to use ApacheHttpClient5Transport instead? Then 3.0 could be built with JDK-8 support and that would offer a path forward for any JDK-8 users.

reta · 2023-02-10T20:05:06Z

This can be considered a breaking change.

Is it feasible to remove the OpenSearch core dependency and RestClientTransport all together in 3.0 and instruct users to use ApacheHttpClient5Transport instead? Then 3.0 could be built with JDK-8 support and that would offer a path forward for any JDK-8 users.

@andrross the plan for deprecation + gradual removal is here opensearch-project/opensearch-java#326

dblock · 2023-02-14T15:00:01Z

I haven't heard anything from any new users about needing JDK8, has anyone? Decoupling brings a lot of other benefits. I propose we close this PR without merging and focus on opensearch-java decoupling (and restoring JDK8 support there with new code).

wbeckler · 2023-02-20T17:39:27Z

To answer @dblock's question, we recently heard a need on the forum for Java 8: https://forum.opensearch.org/t/opensearch-1-3-x-gradle-build-problem-w-java-8/12400/5

The big need for this comes from the opensearch-hadoop situation. Today elasticsearch-hadoop is Java 8 compatible. If we want to integrate opensearch-java into opensearch-hadoop to reduce the overall java surface area, today it would be a breaking change in an environment with lots of legacy Java 8 stuff in the Hadoop and Spark applications that would consume the opensearch-hadoop client.

reta · 2023-02-20T18:12:58Z

To answer @dblock's question, we recently heard a need on the forum for Java 8: https://forum.opensearch.org/t/opensearch-1-3-x-gradle-build-problem-w-java-8/12400/5

Somewhat related story is on Apache Flink side apache/flink-connector-opensearch#4

opensearch-trigger-bot · 2023-07-16T15:22:07Z

This PR is stalled because it has been open for 30 days with no activity. Remove stalled label or comment or this will be closed in 7 days.

opensearch-trigger-bot · 2023-07-24T15:21:53Z

This PR was closed because it has been stalled for 7 days with no activity.

dlvenable requested review from a team and reta as code owners May 4, 2022 14:58

Set the source and target compatibility for the opensearch-rest-clien…

ad6bd4a

…t to Java 8. This project uses a client test library which is now also supporting Java 8. Signed-off-by: David Venable <[email protected]>

dlvenable force-pushed the rest-client-java8-compatibility branch from ef81f58 to ad6bd4a Compare May 4, 2022 15:04

dlvenable mentioned this pull request May 4, 2022

[PROPOSAL] Restore support for Java 8 opensearch-project/opensearch-java#156

Closed

dblock requested changes May 5, 2022

View reviewed changes

Build the test and low-level REST clients using Java 8.

5283072

Signed-off-by: David Venable <[email protected]>

dlvenable mentioned this pull request May 10, 2022

[DRAFT] Build this project and unit test using JDK 8. opensearch-project/opensearch-java#157

Closed

dblock reviewed May 10, 2022

View reviewed changes

opensearch-trigger-bot bot added the stalled Issues that have stalled label Jul 16, 2023

opensearch-trigger-bot bot closed this Jul 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Java 8 for opensearch-rest-client #3181

Support Java 8 for opensearch-rest-client #3181

dlvenable commented May 4, 2022 •

edited

Loading

opensearch-ci-bot commented May 4, 2022

opensearch-ci-bot commented May 4, 2022

reta commented May 4, 2022

dblock commented May 4, 2022

dblock left a comment

opensearch-ci-bot commented May 6, 2022

dblock left a comment

dblock May 10, 2022

wbeckler commented Feb 9, 2023

reta commented Feb 10, 2023

dlvenable commented Feb 10, 2023

reta commented Feb 10, 2023

andrross commented Feb 10, 2023

reta commented Feb 10, 2023 •

edited

Loading

andrross commented Feb 10, 2023

reta commented Feb 10, 2023

dlvenable commented Feb 10, 2023

andrross commented Feb 10, 2023

reta commented Feb 10, 2023

dblock commented Feb 14, 2023

wbeckler commented Feb 20, 2023

reta commented Feb 20, 2023

opensearch-trigger-bot bot commented Jul 16, 2023

opensearch-trigger-bot bot commented Jul 24, 2023

Support Java 8 for opensearch-rest-client #3181

Support Java 8 for opensearch-rest-client #3181

Conversation

dlvenable commented May 4, 2022 • edited Loading

Description

Issues Resolved

Verification

Check List

opensearch-ci-bot commented May 4, 2022

opensearch-ci-bot commented May 4, 2022

reta commented May 4, 2022

dblock commented May 4, 2022

dblock left a comment

Choose a reason for hiding this comment

opensearch-ci-bot commented May 6, 2022

dblock left a comment

Choose a reason for hiding this comment

dblock May 10, 2022

Choose a reason for hiding this comment

wbeckler commented Feb 9, 2023

reta commented Feb 10, 2023

dlvenable commented Feb 10, 2023

reta commented Feb 10, 2023

andrross commented Feb 10, 2023

reta commented Feb 10, 2023 • edited Loading

andrross commented Feb 10, 2023

reta commented Feb 10, 2023

dlvenable commented Feb 10, 2023

andrross commented Feb 10, 2023

reta commented Feb 10, 2023

dblock commented Feb 14, 2023

wbeckler commented Feb 20, 2023

reta commented Feb 20, 2023

opensearch-trigger-bot bot commented Jul 16, 2023

opensearch-trigger-bot bot commented Jul 24, 2023

dlvenable commented May 4, 2022 •

edited

Loading

reta commented Feb 10, 2023 •

edited

Loading