-
-
Notifications
You must be signed in to change notification settings - Fork 246
HSEARCH-5464 Pluggable rest clients in the elasticsearch backend #4798
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
HSEARCH-5464 Pluggable rest clients in the elasticsearch backend #4798
Conversation
aac26ce to
1d41060
Compare
3da3726 to
610c554
Compare
e91fd98 to
ae10272
Compare
marko-bekhta
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @yrodiere @lucamolteni 🙂 👋🏻
I've added some comments here and there in this PR in places that may be of interest.
Otherwise there's a lot of "noise", from moving packages and renaming.
This PR doesn't include the docs+migration guide updates. I was thinking about sending that separately, as this already has a lot of changes 🙂.
...java/org/hibernate/search/backend/elasticsearch/client/common/spi/ElasticsearchResponse.java
Outdated
Show resolved
Hide resolved
...ain/java/org/hibernate/search/backend/elasticsearch/client/common/gson/spi/GsonProvider.java
Show resolved
Hide resolved
...bernate/search/backend/elasticsearch/client/ElasticsearchHttpClientConfigurationContext.java
Show resolved
Hide resolved
...tiontest/backend/elasticsearch/client/ElasticsearchClientFactoryImplElasticsearchJavaIT.java
Show resolved
Hide resolved
...te/search/backend/elasticsearch/client/java/ElasticsearchHttpClientConfigurationContext.java
Outdated
Show resolved
Hide resolved
...search/backend/elasticsearch/client/common/cfg/ElasticsearchBackendClientCommonSettings.java
Outdated
Show resolved
Hide resolved
...e/search/backend/elasticsearch/client/common/spi/ElasticsearchRequestInterceptorContext.java
Show resolved
Hide resolved
...ernate/search/backend/elasticsearch/client/rest5/impl/ClientRest5HttpRequestInterceptor.java
Show resolved
Hide resolved
...a/org/hibernate/search/backend/elasticsearch/client/opensearch/rest/impl/GsonHttpEntity.java
Outdated
Show resolved
Hide resolved
...ava/org/hibernate/search/backend/elasticsearch/client/ElasticsearchHttpClientConfigurer.java
Show resolved
Hide resolved
e347a5f to
2e6dca2
Compare
yrodiere
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't review it all but I went through your comments and a bit more, here's what I have to say :)
It's really a shame we can't have a dependency from clients to backend...
This PR doesn't include the docs+migration guide updates. I was thinking about sending that separately, as this already has a lot of changes 🙂.
Since the docs changes would be in very isolated files, you might as well add these changes here :)
...ain/java/org/hibernate/search/backend/elasticsearch/client/common/gson/spi/GsonProvider.java
Show resolved
Hide resolved
...te/search/backend/elasticsearch/client/java/ElasticsearchHttpClientConfigurationContext.java
Outdated
Show resolved
Hide resolved
...java/org/hibernate/search/backend/elasticsearch/client/common/spi/ElasticsearchResponse.java
Outdated
Show resolved
Hide resolved
...c/main/java/org/hibernate/search/backend/elasticsearch/impl/ElasticsearchBackendFactory.java
Outdated
Show resolved
Hide resolved
...c/main/java/org/hibernate/search/backend/elasticsearch/impl/ElasticsearchBackendFactory.java
Outdated
Show resolved
Hide resolved
...search/backend/elasticsearch/client/common/cfg/ElasticsearchBackendClientCommonSettings.java
Outdated
Show resolved
Hide resolved
|
Thanks for having a look!
ohh I already did 🫣 🙂 here: at the time I didn't want to prevent anyone from looking at the PR waiting for the docs, but as I had some "free time" since then 😁 I managed to push the change here 😄 |
I envy your time management skills. Honestly. |
yrodiere
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some more comments on the docs. Looks great :)
documentation/src/main/asciidoc/public/reference/_backend-elasticsearch.adoc
Outdated
Show resolved
Hide resolved
d4e3fce to
b805101
Compare
since the behavior of this client is slightly different
…plement the clients as dependencies
…he backend and remove the client-common
3152541 to
20de88e
Compare
203a90a to
d32db4d
Compare
|


https://hibernate.atlassian.net/browse/HSEARCH-5464
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.