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

Downgrade client to build with Java 8 #371

Closed

Conversation

harshavamsi
Copy link
Contributor

Description

Coming from #156, this downgrades the target to Java 8. This has to go in after #326.

Issues Resolved

Closes #156

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.

Signed-off-by: Harsha Vamsi Kalluri <[email protected]>
@dlvenable
Copy link
Member

dlvenable commented Feb 21, 2023

Nice! Thanks @harshavamsi !

Ideally, this project should also include the Java 8 in the build/test Java version matrix.

strategy:
matrix:
java: [ 11 ]

I think ideally all the tests would run on Java 8 and 11. But, some of the integration tests require Java 11 due to the OpenSearch test framework. Maybe the project could run the build and unit tests on Java 8/11. And then run only integration tests on Java 11.

@reta
Copy link
Collaborator

reta commented Feb 21, 2023

Nice! Thanks @harshavamsi !

Ideally, this project should also include the Java 8 in the build/test Java version matrix.

strategy:
matrix:
java: [ 11 ]

I think ideally all the tests would run on Java 8 and 11. But, some of the integration tests require Java 11 due to the OpenSearch test framework. Maybe the project could run the build and unit tests on Java 8/11. And then run only integration tests on Java 11.

@dlvenable we cannot do that now, the opensearch-java uses test-framework from core - this is still JDK-11, so we either drop this dependency (copy tons of stuff) or use JDK-11+ for compile / build / test

@dlvenable
Copy link
Member

Nice! Thanks @harshavamsi !
Ideally, this project should also include the Java 8 in the build/test Java version matrix.

strategy:
matrix:
java: [ 11 ]

I think ideally all the tests would run on Java 8 and 11. But, some of the integration tests require Java 11 due to the OpenSearch test framework. Maybe the project could run the build and unit tests on Java 8/11. And then run only integration tests on Java 11.

@dlvenable we cannot do that now, the opensearch-java uses test-framework from core - this is still JDK-11, so we either drop this dependency (copy tons of stuff) or use JDK-11+ for compile / build / test

This is probably beyond the scope of this PR, but we may wish to split the integration tests (which use the test-framework) from unit tests.

@dblock
Copy link
Member

dblock commented Feb 28, 2023

If we can run some basic tests with JDK8 I'd be satisfied. We can then run the full suite with JDK11 and have more work items about increasing test coverage on JDK8 by moving frameworks. Hope you can finish this @harshavamsi!

Signed-off-by: Harsha Vamsi Kalluri <[email protected]>
@rgopidiaws
Copy link

can you please prioritize this? we are currently blocked on this as we have a service which is using JDK8 and we are not able to use this client in that service. Appreciate it if you can prioritize this.

@dblock
Copy link
Member

dblock commented Jun 20, 2023

@rgopidiaws This hasn't been touched for a while so I am afraid nobody is working on this right now. Want to give it a try?

@dblock
Copy link
Member

dblock commented Dec 4, 2023

@harshavamsi Any interest in re-picking this up on your copious free time (TM)? :)

@VachaShah
Copy link
Collaborator

Closing this PR since restoring support for Java 8 is done.

@VachaShah VachaShah closed this Feb 27, 2024
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.

[PROPOSAL] Restore support for Java 8
6 participants