Extracted builder (or json request generation) classes to libs with minimal dependencies #30791
Comments
|
See also #29184 |
|
Yes, I'm aware of the transitive dependency of high level client on Lucene. That why we were considering to use low level rest client. Still, for queries we have to use classes from ES And thus could not be easily isolated from the rest of the server code, unfortunately. |
|
Pinging @elastic/es-core-infra |
|
As mentioned in #29184 (comment) this is definitely a goal of the High Level REST client (to separate its dependencies from the server dependencies. However this is a lot of work and it will take us time to achieve it as the dependencies are complex and some are used in the client code itself which we would need to unpick. Unfortunately its not as easy as moving classes to a new jar. I'm going to close this issue in favour of #29184 since we already have discussion on this there. |
lucamilanesio
pushed a commit
to GerritCodeReview/gerrit
that referenced
this issue
May 28, 2018
Remove jest API which depends on Elasticsearch native API (also removed), so no longer depend on lucene for this scope. Replace such removed dependencies with the Elasticsearch low-level API, which does not depend on Lucene. That now used API is a REST client with minimal dependencies, indeed. -As opposed to Elastic's current high-level API, which still depends on lucene and other Gerrit-constraining libraries. Do so in order to decouple the elasticsearch client from lucene in Gerrit. That REST client does not logically require Lucene anyway. Most importantly, such decoupling now enables upcoming support for more than just one Elasticsearch server version. This includes the new possibility of bumping the latter to multiple yet later versions, such as 5 and 6.x. The currently used version (2.4) should also still be kept, for a while. Bumping such versions will likely require some Elasticsearch client code adaptations, so that Gerrit can (dynamically?) switch between either Elasticsearch server version to eventually support (hopefully soon). Doing the same for the elasticsearch tests in Gerrit is to be done using another change or more changes. Add a partial and customized fork of [1], based on [1]'s commit [2], to preserve the ability of building proper json requests for Elasticsearch. Put that forked json-generating code under a new 'builders' sub-package. There should be a possibility in some near future to consider removing that fork, based on potential progress such as the one proposed in [3]. Meanwhile, this fork shall be maintained to usual Gerrit quality levels. [1] https://github.com/elastic/elasticsearch [2] tag: v2.4.4 [3] elastic/elasticsearch#30791 Bug: Issue 6094 Change-Id: I720c9885c9eab2388acc328eecb9eaa6940ced0c
lucamilanesio
pushed a commit
to GerritCodeReview/gerrit
that referenced
this issue
Jun 4, 2018
Remove jest API which depends on Elasticsearch native API (also removed), so no longer depend on lucene for this scope. Replace such removed dependencies with the Elasticsearch low-level API, which does not depend on Lucene. That now used API is a REST client with minimal dependencies, indeed. -As opposed to Elastic's current high-level API, which still depends on lucene and other Gerrit-constraining libraries. Do so in order to decouple the elasticsearch client from lucene in Gerrit. That REST client does not logically require Lucene anyway. Most importantly, such decoupling now enables upcoming support for more than just one Elasticsearch server version. This includes the new possibility of bumping the latter to multiple yet later versions, such as 5 and 6.x. The currently used version (2.4) should also still be kept, for a while. Bumping such versions will likely require some Elasticsearch client code adaptations, so that Gerrit can (dynamically?) switch between either Elasticsearch server version to eventually support (hopefully soon). Doing the same for the elasticsearch tests in Gerrit is to be done using another change or more changes. Add a partial and customized fork of [1], based on [1]'s commit [2], to preserve the ability of building proper json requests for Elasticsearch. Put that forked json-generating code under a new 'builders' sub-package. There should be a possibility in some near future to consider removing that fork, based on potential progress such as the one proposed in [3]. Meanwhile, this fork shall be maintained to usual Gerrit quality levels. [1] https://github.com/elastic/elasticsearch [2] tag: v2.4.4 [3] elastic/elasticsearch#30791 Bug: Issue 6094 Change-Id: I720c9885c9eab2388acc328eecb9eaa6940ced0c
lucamilanesio
pushed a commit
to lucamilanesio/index-elasticsearch
that referenced
this issue
May 13, 2022
Remove jest API which depends on Elasticsearch native API (also removed), so no longer depend on lucene for this scope. Replace such removed dependencies with the Elasticsearch low-level API, which does not depend on Lucene. That now used API is a REST client with minimal dependencies, indeed. -As opposed to Elastic's current high-level API, which still depends on lucene and other Gerrit-constraining libraries. Do so in order to decouple the elasticsearch client from lucene in Gerrit. That REST client does not logically require Lucene anyway. Most importantly, such decoupling now enables upcoming support for more than just one Elasticsearch server version. This includes the new possibility of bumping the latter to multiple yet later versions, such as 5 and 6.x. The currently used version (2.4) should also still be kept, for a while. Bumping such versions will likely require some Elasticsearch client code adaptations, so that Gerrit can (dynamically?) switch between either Elasticsearch server version to eventually support (hopefully soon). Doing the same for the elasticsearch tests in Gerrit is to be done using another change or more changes. Add a partial and customized fork of [1], based on [1]'s commit [2], to preserve the ability of building proper json requests for Elasticsearch. Put that forked json-generating code under a new 'builders' sub-package. There should be a possibility in some near future to consider removing that fork, based on potential progress such as the one proposed in [3]. Meanwhile, this fork shall be maintained to usual Gerrit quality levels. [1] https://github.com/elastic/elasticsearch [2] tag: v2.4.4 [3] elastic/elasticsearch#30791 Bug: Issue 6094 Change-Id: I720c9885c9eab2388acc328eecb9eaa6940ced0c
lucamilanesio
pushed a commit
to lucamilanesio/index-elasticsearch
that referenced
this issue
May 13, 2022
Remove jest API which depends on Elasticsearch native API (also removed), so no longer depend on lucene for this scope. Replace such removed dependencies with the Elasticsearch low-level API, which does not depend on Lucene. That now used API is a REST client with minimal dependencies, indeed. -As opposed to Elastic's current high-level API, which still depends on lucene and other Gerrit-constraining libraries. Do so in order to decouple the elasticsearch client from lucene in Gerrit. That REST client does not logically require Lucene anyway. Most importantly, such decoupling now enables upcoming support for more than just one Elasticsearch server version. This includes the new possibility of bumping the latter to multiple yet later versions, such as 5 and 6.x. The currently used version (2.4) should also still be kept, for a while. Bumping such versions will likely require some Elasticsearch client code adaptations, so that Gerrit can (dynamically?) switch between either Elasticsearch server version to eventually support (hopefully soon). Doing the same for the elasticsearch tests in Gerrit is to be done using another change or more changes. Add a partial and customized fork of [1], based on [1]'s commit [2], to preserve the ability of building proper json requests for Elasticsearch. Put that forked json-generating code under a new 'builders' sub-package. There should be a possibility in some near future to consider removing that fork, based on potential progress such as the one proposed in [3]. Meanwhile, this fork shall be maintained to usual Gerrit quality levels. [1] https://github.com/elastic/elasticsearch [2] tag: v2.4.4 [3] elastic/elasticsearch#30791 Bug: Issue 6094 Change-Id: I720c9885c9eab2388acc328eecb9eaa6940ced0c
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Using the Elasticsearch low-level Java API or rest client is convenient because of the minimal dependencies that library has. It would be as useful (if not essential) for integrations like ours to benefit from minimal dependencies as well for the Builder classes.
A similar, related extraction was completed recently for XContent: #28504. How about considering doing the same for Builder classes in packages such as index.query, action.support, search.builder and maybe more? Those classes work closely with XContentBuilder thus could benefit from a similar extraction treatment.
Of course, the sibling high-level API aims at a similar purpose, but with more dependencies which conflict with libraries that are already integrated. That approach also prevents a system embedding an Elasticsearch client from supporting multiple Elasticsearch server versions.
The text was updated successfully, but these errors were encountered: