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

Jest Implementation [DATAES-220] #793

Closed
spring-projects-issues opened this issue Dec 12, 2015 · 35 comments
Closed

Jest Implementation [DATAES-220] #793

spring-projects-issues opened this issue Dec 12, 2015 · 35 comments
Labels
has: votes-jira in: repository Repositories abstraction status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link

spring-projects-issues commented Dec 12, 2015

Julien Roy opened DATAES-220 and commented

Hi,

I work actually on projet with Spring Data Elasticsearch and AWS Elasticsearch.
The AWS ES implementation like many other cloud provider support only the HTTP protocol of ES.

I developed à minimal implementation of Spring Data ES backend by the Jest Library.
It's essentially an implementation of ElasticsearchOperations with Jest API.
It's not totally complete but it's work well in production.

Are you interested for integrate an Jest implementation into Spring Data Elasticsearch ?
And would you like that i clean my code and i make a "pull request" with this minimal implementation ?

Julien


Affects: 1.3.1 (Gosling SR1)

Referenced from: pull request #144

15 votes, 26 watchers

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Dec 15, 2015

Mohsin Husen commented

Thanks for contacting for this feature.

There is a huge demand to use http port while using spring data elasticsearch library either stand alone or with spring boot.

We would love to have JEST implementation in spring data elasticsearch.

Please fill free to open a pull request

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Dec 16, 2015

Julien Roy commented

Ok thanks, i will be starting to work on this PR.

Do you have any whishes about ES version compatibility ?

Indeed Jest maintain two version :

1.X.X : Compatible with ES 1.7
2.X.X : Compatible with ES 2.0 ( and i don't know if it's working with lesser than ES 2.0 )

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Dec 22, 2015

Mohsin Husen commented

We are actively working to support 2.x version. better to target 2.x as of now.

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Jan 9, 2016

Julien Roy commented

Hi,

My first implementation is visible here :
https://github.com/VanRoy/spring-data-elasticsearch/tree/jest-implementation-2.0

It's based on your branch : issue/DATAES-211.
It pass 50 of 65 tests of ElasticsearchTemplateTests.

Currently it's a very conservative implementation ( no changes on existing classes ).

I will work on another implementation with more factorization of code between ElasticsearchTemplate and my JestElasticsearchTemplate.

I thinking on classes tree like that :

  • ElasticsearchOperations ( base interface that contains common methods )
    -> JestElasticsearchOperations ( Specific methods for Jest backend )
    -> NativeElasticsearchOperations ( Specific methods for Native ES )

  • ElasticsearchTemplate ( base class that contains common code )
    -> JestElasticsearchTemplate ( Specific implementation for Jest backend )
    -> NativeElasticsearchTemplate ( Specific implementation for Native ES )

What do you think about that ?

Thanks for you help.
Julien

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Feb 5, 2016

Petar Tahchiev commented

+1 On this.. I'm not sure why it's a minor. For me this is a blocker

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Feb 13, 2016

Julien Roy commented

Petar, i agree with you.

Mohsin, I would like to complete my work about this feature.
Can you tell me what do you think about my previous message ( refactorization or conservative implementation ).

Thanks a lot.
Julien

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Feb 16, 2016

Mohsin Husen commented

Thanks Julien

We will review the PR soon. and we have working branch for 2.x now.

https://github.com/spring-projects/spring-data-elasticsearch/tree/DATAES-211-ES2.0

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Feb 24, 2016

Mohsin Husen commented

After talking to various Spring Data devs about this feature, all have one common suggestion

To have separate ElasticsearchTemplate for Jest is ok, as you need to have java code which deals with http calls using http client.

but to add one more mapper for GSON can be problematic, as Spring Data elasticsearch already have jackson for mapping in and out from elasticsearch.

Also adding additional third party library we will be bound to releases of other libraries too.

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Feb 25, 2016

Robert Gründler commented

Thanks Julien, this looks great! We would like to test the jest support PR, is there
a way to do this without getting the source?

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Feb 25, 2016

Mohsin Husen commented

To add above, i have created a branch

https://github.com/spring-projects/spring-data-elasticsearch/tree/DATAES-220

please propose a PR to this and we will accept and add to build pipeline so that we have stable version where end user can test the feature.

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Feb 25, 2016

Milan Agatonovic commented

I am very interested in JEST implementation merged.
Is it still planned for Hopper RC1 release, 1st week of March?

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Mar 1, 2016

Robert Gründler commented

Julien, should the branch in your fork compile without errors? We're trying to push it in our private repo in the meanwhile for testing, but see some compilation errors, mostly regarding scroll:

"(cannot infer type-variable(s) T
(argument mismatch; <anonymous org.springframework.data.elasticsearch.core.JestSearchResultMapper> cannot be converted to org.springframework.data.elasticsearch.core.SearchResultMapper))
method org.springframework.data.elasticsearch.core.JestElasticsearchTemplate.<T>scroll(java.lang.String,long,org.springframework.data.elasticsearch.core.JestScrollResultMapper) is not applicable"

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Mar 3, 2016

Mohsin Husen commented

not possible to add it in next release(Hopper RC1), as we still don't have working branch.

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Mar 14, 2016

Robert Gründler commented

i've been looking into getting this branch to work here https://github.com/pulse00/spring-data-elasticsearch/tree/jest-implementation-2.0 .

I'm not sure if the original author has still time to finish this PR - if not our company would like to offer help implementing the currently unimplemented parts (mainly faceted search what i've seen and some scroll-mapping issues).

Julien, would you be ok with this?

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Mar 14, 2016

Julien Roy commented

Robert,

Indeed, I did not have the time to work on this feature this weeks.
I will be very happy to share the work with other people.

At this times, two question will be on hold :

  1. Did you make an abstract ElasticsearchTemplate for share code between NativeElasticsearchTemplate and JestElasticsearchTemplate or just duplicate like i have doing in the current implementation ?

  2. The Jest client library come with GSON dependency and Spring Data ES use Jackson. How solve this problem ?
    By contributing to current experimentation on Jest that move from GSON to Jackson (https://github.com/ctrimble/Jest/tree/experiment_object-mapper)
    or
    By remove Jest and re-implementing all HTTP calls.

I waiting for responses from Mohsin about this questions and continue my work after.
Julien

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Mar 15, 2016

Robert Gründler commented

Regarding the ElasticsearchTemplate: I think we might opt for a similar approach like spring-data-redis. There you have the option of multiple underlying clients without having to change the template:

The main concepts there involve connection factory and a connection interface:

Maybe this is a viable approach?

Regarding the gson/jackson issue: Re-Implementing jest doesn't sound very appealing to me, so maybe we can get in touch with the jest people and check the status of the gson refactoring?

-robert

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Mar 20, 2016

Raunak Gupta commented

One of the reason why I moved to Spring Elastic Search was because it used Jackson for JSON parsing. Looks like GSON won't leave me alone :D
Re Jest moving to Jackson, this has been ongoing for a while now (searchbox-io/Jest#45). I don't think it is happening anytime soon, and I rather have Spring continue its use of Jackson. Not sure if the rest of the community agrees with me..

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Mar 30, 2016

Petar Tahchiev commented

Any news here? What is the bottleneck of merging this?

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Apr 20, 2016

Robert Gründler commented

I will have time this week to work on the pull request. Fyi: I'll use jest as-is (meaning with the jackson dependency).

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Apr 21, 2016

Robert Gründler commented

I have merged the latest spring-data-elasticsearch master branch into the jest fork, changes can be found here: https://github.com/pulse00/spring-data-elasticsearch/tree/feature/jest

To finish this there are 2 options:

Option #1: Keep a single implementation of ElasticsearchTemplate working with both the elasticsearch and the jest client

This option would require some substantial refactoring to be done, most likely with breaking backwards compatibility.
The reason is that ElasticSearchOperations and the classes used in the interface signatures are pretty much tied to
the elasticsearch client implementation.

Here's a couple of examples:

Especially the *ResultMapper classes rely on the ES client API.

If introducing breaking changes is ok for the core committers, i can work on a refactoring to

  • Remove ES specific API from the ElasticsearchOperations interface
  • Refactor the Mapper and Search classes to remove the ES specific api

Option #2: Keep the current implementation (2 separate implementations of ElasticsearchOperations)

There would be 2 implementations:

  1. The existing ElasticSearchTemplate
  2. The new JestElasticSearchTemplate

Existing project would not require any changes, but the new JestElasticSearchTemplate would have some methods
throw an UnsupportedOperationException because the interface signature is incompatible with the jest mapping / search results.

Examples:

Any feedback on these 2 options would be greatly appreciated.

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Apr 21, 2016

Robert Gründler commented

Here's my pull request with option #2 from the above comment.

#144

This commit requires the next patchlevel version of jest though, because of searchbox-io/Jest#340.

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Jun 30, 2016

Julien Roy commented

Hi Moshin,

Do you see any inconveniences to I package my JestElasticSearchTemplate in new and independent Github projet and in the Maven Central ?

You can see the projet in this Github repository : https://github.com/VanRoy/spring-data-jest
I will waiting for your agreement to publish to maven central.

Thanks.
Julien

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Jul 8, 2016

Julien Roy commented

Hi Moshin,

Finally, I have published the artefact on maven central.

Do not hesitate to come back to me if any problems related to this publicated.

Thank a lot.
Julien

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Jul 10, 2016

Amin Mohammed-Coleman commented

Is the Jest implementation the only planned implementation for talking to ES via Http? ES have released (alpha) of a RestClient. Will this be used as an alternative to Jest?

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Jul 15, 2016

Terence Mill commented

I read a notice on spring blog that there i now support for Jest . Does it mean this has been merged to a release ? Which one , because the most current on spring repo is still the old spring-data-elasticsearch 1.1.0

https://spring.io/blog/2016/07/12/this-week-in-spring-july-12th-2016

The first release candidate of Spring Boot 1.4 is now available, packed with a lot of great things! This new release features, among other things, a unified @EntityScan for JPA, MongoDB, Neo4j, Couchbase and Cassandra, an auto-configured RestTemplateBuilder, support for pure REST client tests via @RestClientTest, support for Jest (the Elasticsearch REST client), and upgrades to Spring Integration 4.3, Spring AMQP 1.6, Spring REST Docs 1.1, MongoDB Java Driver 3 and more! Download the bits and kick the tires right now!

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Jul 15, 2016

Mohsin Husen commented

@Terence, blog you are referring to is the news about Spring Boot supporting Jest client, not in the actual spring data elasticsearch.
You can get more info at spring-projects/spring-boot#6032

@Amin : Yes once elasticsearch 5.x is released we are planning to support Native Rest client.

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Aug 9, 2016

Joseph Blair commented

@Mohsin: Are you planning to release Julien's Jest implementation in the meantime while waiting for Elasticsearch 5.x? Or is it being scrapped in favor of moving directly to the native REST client? Just wondering what all of this means in terms of how long we'll need to wait before this feature is actually usable in production.

Also, would this driver be compatible with the AWS Elasticsearch Service, or does Amazon need to support 5.x first?

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Aug 11, 2016

Julien Roy commented

@Joseph In waiting of decision, you can already use my implementation ( https://github.com/VanRoy/spring-data-jest ).
It's available on maven central, works well on AWS with ES 1.5 and 2.3.
It's compatible with Spring Boot 1.3, 1.4 and Spring Data 1.3 and 2.0.

Julien

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Mar 3, 2017

Fabio Maffioletti commented

@Julien what is the status of this? Is it planned to be integrate any time soon? btw I tried to use your existing implementation but there are (small) issues (already pointed out on Github) that prevent a correct functionality using the latest elastic search. When do you think you will release your library? Thanks

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Mar 3, 2017

Julien Roy commented

Hi @Fabio,

In my opinion, I think that it will never be integrated in master Spring Data.
The privilegied option is to use Native Rest client of ES 5.0.

In waiting you can also use my Implementation : https://github.com/VanRoy/spring-data-jest
I have planned the next release for the next week.

Julien

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Jun 1, 2017

Petar Tahchiev commented

Any news here? Can someone please share their thoughts on this? Will it ever make it into spring-data-elasticsearch? What is the stopper here - elasticsearch 6.0.0-alpha1 is already out.

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Oct 25, 2018

Tobias Günther commented

It's a real bummer that there is no progress here. A missing native support is almost a bug, especially with some providers like AWS not offering access to the node ports of the cluster.

Third party solutions are suitable as aworkaround but are not updated frequently enough

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Oct 28, 2018

Don Wellington commented

XyarenI think DATAES-407 should cover what you are looking for

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Apr 3, 2019

sothawo commented

Like Don Wellington said, the ElasicsearcRestTemplate implemented in DATAES-407 uses the RestClient of Elasticsearch. So I think this issue can be closed - the PR it references is closed since May 2016

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Apr 17, 2019

sothawo commented

I resolved this as won't do, as the functionality is contained in the ElasticsearchRestClient, so this change is not needed any more

@spring-projects-issues spring-projects-issues added status: declined A suggestion or change that we don't feel we should currently apply in: repository Repositories abstraction type: enhancement A general enhancement has: votes-jira labels Dec 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has: votes-jira in: repository Repositories abstraction status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

1 participant