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

[FEATURE] Add opensearch-java client support #19

Closed
reta opened this issue Oct 11, 2022 · 17 comments · Fixed by #227
Closed

[FEATURE] Add opensearch-java client support #19

reta opened this issue Oct 11, 2022 · 17 comments · Fixed by #227
Assignees
Labels
enhancement New feature or request v1.4.0 Issues and PRs related to version 1.4.0

Comments

@reta
Copy link
Collaborator

reta commented Oct 11, 2022

Is your feature request related to a problem?

The only supported client at the moment is RHLC, we should also add opensearch-java support.

What solution would you like?

Support opensearch-java

What alternatives have you considered?

Keep only RHLC

Do you have any additional context?

N/A

@lbenedetto
Copy link

https://opensearch.org/docs/latest/clients/java-rest-high-level/

The OpenSearch Java high-level REST client will be deprecated starting with OpenSearch version 3.0.0 and will be removed in a future release. We recommend switching to the Java client instead.

@johnament
Copy link

hi @reta it's been a while, hope you're doing well.

Was wondering your thoughts on this issue. Now that high level rest client is deprecated, do you think it makes more sense to just update this code to use opensearch-java or make both options until 3.0 fully removes the deprecated client?

I might have some cycles to look at this.

@reta
Copy link
Collaborator Author

reta commented Oct 2, 2023

Hey @johnament , great to hear from you!

Was wondering your thoughts on this issue. Now that high level rest client is deprecated, do you think it makes more sense to just update this code to use opensearch-java or make both options until 3.0 fully removes the deprecated client?

I think it makes sense to go with opensearch-java all way down since Spring Data Elasticsearch 5.2 drops the rest client support altogether, and we won't be able to update.

@reta
Copy link
Collaborator Author

reta commented Nov 7, 2023

@johnament it seems like the urgency for this one has elevated a bit, AFAIK Spring Data Elasticsearch in 5.2 drops the RHCL based support completely, have you started some work already? If not, I could kick it off and would appreciate your help, thank you.

@sothawo
Copy link

sothawo commented Nov 14, 2023

We are removing the Elasticsearch RestHighLevelClient, but your code is derived from the abstract class that is the base for the old and new client, so this removal should not affect you.

@rursprung
Copy link

rursprung commented Jan 5, 2024

is there a plan for when this will happen? it'd be great to get rid of the RHLC dependency when working with spring-data-opensearch

@reta
Copy link
Collaborator Author

reta commented Jan 5, 2024

is there a plan for when this will happen?

There is intent to do that, no timelines though, sadly

@johnament
Copy link

@reta apologies. no work has started on my side as we are able to do what we need using opensearch-java directly instead of the repository pattern though I have more use cases I want to enable that support repositories and raw client usage and would see this being ideal. I am available to help out a bit but have just now started digging back in.

@reta
Copy link
Collaborator Author

reta commented Jan 7, 2024

I am available to help out a bit but have just now started digging back in.

No problem, thanks a lot for the update @johnament , I hope to pick it up shortly (at least have a draft pull request to collaborate), thank you!

@johnament
Copy link

Also, I'm sure you already realize it but when cutting over I found it easier to start with something like this, which constructs the java client from the underlying rest client via transport, this way the configuration code can be cutover separately from the consuming code:

diff --git a/settings.gradle.kts b/settings.gradle.kts
index 2180e9b..c1df11d 100644
--- a/settings.gradle.kts
+++ b/settings.gradle.kts
@@ -31,6 +31,7 @@ dependencyResolutionManagement {
     
     create("opensearchLibs") {
       version("opensearch", "2.11.0")
+      library("java", "org.opensearch.client:opensearch-java:2.8.1")
       library("client", "org.opensearch.client", "opensearch-rest-client").versionRef("opensearch")
       library("high-level-client", "org.opensearch.client", "opensearch-rest-high-level-client").versionRef("opensearch")
       library("sniffer", "org.opensearch.client", "opensearch-rest-client-sniffer").versionRef("opensearch")
diff --git a/spring-data-opensearch/build.gradle.kts b/spring-data-opensearch/build.gradle.kts
index a981afe..24217e0 100644
--- a/spring-data-opensearch/build.gradle.kts
+++ b/spring-data-opensearch/build.gradle.kts
@@ -26,6 +26,7 @@ dependencies {
   api(opensearchLibs.high.level.client) {
     exclude("commons-logging", "commons-logging")
   }
+  api(opensearchLibs.java)
   
   implementation(jacksonLibs.core)
   implementation(jacksonLibs.databind)
diff --git a/spring-data-opensearch/src/main/java/org/opensearch/data/client/orhlc/RestClients.java b/spring-data-opensearch/src/main/java/org/opensearch/data/client/orhlc/RestClients.java
index 5a84f7a..95c0003 100644
--- a/spring-data-opensearch/src/main/java/org/opensearch/data/client/orhlc/RestClients.java
+++ b/spring-data-opensearch/src/main/java/org/opensearch/data/client/orhlc/RestClients.java
@@ -30,6 +30,10 @@ import org.apache.http.protocol.HttpContext;
 import org.opensearch.client.RestClient;
 import org.opensearch.client.RestClientBuilder;
 import org.opensearch.client.RestHighLevelClient;
+import org.opensearch.client.json.jackson.JacksonJsonpMapper;
+import org.opensearch.client.opensearch.OpenSearchClient;
+import org.opensearch.client.transport.OpenSearchTransport;
+import org.opensearch.client.transport.rest_client.RestClientTransport;
 import org.springframework.data.elasticsearch.support.HttpHeaders;
 import org.springframework.util.Assert;
 
@@ -139,6 +143,11 @@ public final class RestClients {
             return rest().getLowLevelClient();
         }
 
+        default OpenSearchClient javaClient() {
+            final OpenSearchTransport transport = new RestClientTransport(lowLevelRest(), new JacksonJsonpMapper());
+            return new OpenSearchClient(transport);
+        }
+
         @Override
         default void close() throws IOException {
             rest().close();

@reta
Copy link
Collaborator Author

reta commented Jan 7, 2024

Also, I'm sure you already realize it but when cutting over I found it easier to start with something like this, which constructs the java client from the underlying rest client via transport, this way the configuration code can be cutover separately from the consuming code:

Thanks @johnament , to be fair I have not considered this option (primarily because it keeps dependency on opensearch-rest-client whereas opensearch-java does not need it with Apache HttpClient 5 based transport), but this is certainly one of the possible route to explore.

@johnament
Copy link

but this is certainly one of the possible route to explore.

So yeah, I had incorrectly assumed that the OpenSearchRestClient was used more than it actually is, and the real problem here is the mapping to/from the internal Spring Data Elasticsearch classes.

Now that there's starting to be bigger forking behaviors here, is that still the right approach to think of? For many of the older models they just maintained arbitrary maps. The new java client now uses more concrete classes.

@reta
Copy link
Collaborator Author

reta commented Jan 7, 2024

Now that there's starting to be bigger forking behaviors here, is that still the right approach to think of? For many of the older models they just maintained arbitrary maps. The new java client now uses more concrete classes.

Correct, the idea was to follow the similar approach Spring Data Elasticsearch took with 2 independent clients for Elasticsearch: RestClient and elasticsearch-java client. So yeah, it is larger change.

@johnament
Copy link

So how would you see handling something like this?

In Opensearch, Alias has 3 different routing strings - https://github.com/opensearch-project/opensearch-java/blob/main/java-client/src/main/java/org/opensearch/client/opensearch/indices/Alias.java#L52-L68 (the extra field has a different semantic per https://opensearch.org/docs/latest/api-reference/index-apis/alias/ )
But in Elastcisearch (and Spring Data ES) there's only two: https://github.com/spring-projects/spring-data-elasticsearch/blob/main/src/main/java/org/springframework/data/elasticsearch/core/index/AliasData.java#L27-L32

Basically my fundamental question is should Spring Data Opensearch continue to be derived from Spring Data Elasticsearch or should it be its own independent module (so that incorrect classes don't leak).

@reta
Copy link
Collaborator Author

reta commented Jan 7, 2024

Basically my fundamental question is should Spring Data Opensearch continue to be derived from Spring Data Elasticsearch or should it be its own independent module (so that incorrect classes don't leak).

This is very good question, the abstractions provided by Spring Data Elasticsearch allowed us to cut a lot of corners, but the key to that was the fact that ES and OS APIs didn't diverge too much (if at all). I think the vision for a long term is that Spring Data Opensearch will become independent.

@reta reta self-assigned this Jan 19, 2024
@AntCode97
Copy link

Hello. @reta
My team has recently been thinking about adopting Opensearch and have been writing some simple code.
However, we realized that RestHighLevelClient is deprecated, so we decided to develop using only opensearch-java

But it seems that only RestHighLevelClient is still supported in spring-data-opensearch,
so we are having a hard time developing with Opensearch.
If this can be resolved quickly, I would like to proceed with development using both RestHighLevelClient and opensearch-java for now.

@reta
Copy link
Collaborator Author

reta commented Mar 14, 2024

Hey @AntCode97

However, we realized that RestHighLevelClient is deprecated, so we decided to develop using only opensearch-java

👍

But it seems that only RestHighLevelClient is still supported in spring-data-opensearch,

That is correct at the moment, the work is ongoing [1], there are some changes on opensearch-java side that have to go in (to complement Spring Data featureset), so no date here sadly.

so we are having a hard time developing with Opensearch.

Using 2 clients is indeed a bit annoying however if you use spring-data-opensearch, in your application you could use opensearch-java with RestClientTransport that uses the same RestClient as spring-data-opensearch, I believe that should expose only opensearch-java to all other parts of the application

[1] #227

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request v1.4.0 Issues and PRs related to version 1.4.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants