-
Notifications
You must be signed in to change notification settings - Fork 191
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] Get rid of the dependency on OpenSearch core #262
Comments
@reta and @dlvenable - I'd like your thoughts on the problem space, the downsides, and potential solutions. |
The core problem I see is that this dependency couples Java clients to the server implementation. The Java version does not impact Python or JS clients for example. But, the update the JDK 11 in core forced an update on clients. OpenSearch core should be able to update JDK versions based on the needs of system administrators and developers, not based on the needs of clients. Similarly, client developers should be able to update their JDK versions based on their needs and the needs of more complicated projects. The low-level REST client from OpenSearch core is mostly a wrapper around Apache HTTP. It adds some logic that is relevant for communicating with an OpenSearch cluster in general, such as load balancing, error/warning handling, node selection controls. I see a few options:
Option 1)
Some downsides:
Option 2)
Some downsides:
|
I agree with @dlvenable statement that
It indeed does not depend on anything from the core. I would like to suggest a third option:
Cons:
The option 1) is also on the table but I would certainly be against of option 2): the core should have no deps on |
I'm good with Option 3 as well. To clarify, my option 2 was really the same from a module perspective. I was proposing that opensearch-java would produce two jars. They would be developed in the same Git repo and released together. From a module perspective core would not really be affected since it would have the same dependency chain. |
I have a few other thoughts on just removing the dependency. Clarity - We can make a clearer story for client developers. Right now, there are three Java clients. In my opinion, this is a confusing aspect of OpenSearch for end-users, especially client developers. I also experienced something similar when I was using Client Configuration - Using the |
Thanks @dlvenable, I think there is an agreement to recommend
You can do that now with |
This is what I understand as well. I'm suggesting that we go one step further and not even offer these other options externally. These clients could potentially be viewed as internal components. Dropping this dependency helps with that.
This is true as well. But we can tighten up the transport concept some by dropping the low-level client. As a client developer I don't know what I'm getting if I choose Also, what about the basic cluster management that |
I think we derailed a bit from the subject (but in a good sense), just to sum up the on the |
Why should the java client be any different from JS or Python client? It shouldn't. If code duplication is to be had, so what? Either way, the long term plan to avoid code duplication is opensearch-project/opensearch-clients#19 which clarifies the relationship between clients and server, all while avoiding duplication for all clients. I would merge a PR that removes the dependency by duplicating whatever code needs to be duplicated and bumping the major version and creating an UPGRADING.md that explains what to do. It could also restore support for JDK 1.8. Hint ... hint :) |
I think the main difference is that the core uses java client internally (in plugins / modules / tests fe).
I will prototype that |
Agreed
It would be nice to avoid it. But, I think clarity to the end-users is more valuable. So I think code duplication is a good trade-off here.
I think a migration path could be helpful there. That is opensearch-java 2.x adds the duplicated code as a new transport. But it retains the old dependency as an optional transport. Then in the major version 3.x there is no option to use the old dependency. |
@dlvenable what is a scenario in which someone benefits from keeping the old dependency as an optional transport? |
@wbeckler , I only think it is useful for a smoother migration. Minor release (say 2.2) - Adds the new transport and deprecates the old A client developer who uses the library has an opportunity in 2.2 to make these changes before they are removed. They see the deprecation and they have some time to migrate. The alternative is that they want to update to 3.0 and suddenly they have to make the change then. |
Why does removing the dependency affect a user of opensearch-java when we're duplicating the old transport? Is there a use of the client that depends on the source of the transport dependency? I'm asking because I'd like to find a way to remove the core dependency and avoid the major version bump. |
+1 @wbeckler if there's no end-user change we can avoid the major version increment, it looks like we're not even changing the namespace |
@dblock @wbeckler this is a tricky question: we could duplicate old transport as-is, which basically means we would have at least 2
We could keep the possibility to use |
I also opened opensearch-project/OpenSearch#5424. Would love your comments @reta and @dlvenable! |
It makes sense to me: if we drop dependency on core, the plugins may migrate to |
Is your feature request related to a problem?
The opensearch-java library uses
RestClient
from OpenSearch core, bringing in the kitchen sink and causing problems such as #156.What solution would you like?
Get rid of the dependency.
The text was updated successfully, but these errors were encountered: