-
Notifications
You must be signed in to change notification settings - Fork 3k
Shared extension for the Hibernate Search Elasticsearch backend #39416
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
Conversation
|
/cc @gsmet (hibernate-search) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
eab51da to
9ecb4e8
Compare
9ecb4e8 to
dcbbe40
Compare
|
Rebased on main. Ran a few tests locally, and it seems to be working fine... we'll need to check the generated documentation for configuration properties though. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
dcbbe40 to
1d3bb33
Compare
This comment has been minimized.
This comment has been minimized.
fb9e99c to
5bff5be
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
This is a nice improvement! 😃
There are a couple of failing tests because of the missing pu-config, I'll let you decide how you want to address those 😃. Other than that, it looks good!
...in/java/io/quarkus/hibernate/search/orm/elasticsearch/runtime/HibernateSearchConfigUtil.java
Show resolved
Hide resolved
...e/search/backend/elasticsearch/runtime/HibernateSearchBackendElasticsearchRuntimeConfig.java
Show resolved
Hide resolved
...te/search/backend/elasticsearch/deployment/HibernateSearchBackendElasticsearchProcessor.java
Outdated
Show resolved
Hide resolved
...kus/hibernate/search/orm/elasticsearch/deployment/HibernateSearchElasticsearchProcessor.java
Show resolved
Hide resolved
5bff5be to
44d8359
Compare
|
Thanks for your comments @marko-bekhta , I think I addressed most of them. Now, the failure to generate documentation is actually more serious than it seemed... I started working on a potential solution, but in essence we're lacking some features in Quarkus docs generation. I'll try to discuss this with Guillaume, see #43808 for details (or don't, and save your time for something more interesting xD). |
This comment has been minimized.
This comment has been minimized.
|
🎊 PR Preview 7ab2138 has been successfully built and deployed to https://quarkus-pr-main-39416-preview.surge.sh/version/main/guides/
|
This comment has been minimized.
This comment has been minimized.
e5a7335 to
fb98664
Compare
|
#43943 was merged, I rebased on main, so I think we're all set. @marko-bekhta I think I addressed your comments, do you mind approving please? |
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.
🎉
|
I'll have a look later today. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
gsmet
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.
Sorry for the very late review. I added only one (annoying, sorry...) comment. I think it would be worth taking it into account even if I know you won't be very excited about it :).
fb98664 to
1c43f90
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 renamed the extension to quarkus-hibernate-search-backend-elasticsearch-common as requested.
Status for workflow
|
Status for workflow
|
|
I somehow forgot about this one... |
|
No harm done: so did I... :) |

To reduce code duplication.
Opening as draft, because:It's based on Refresh documentation (and some tests) of the Hibernate Search + ORM extension #39413 and Extension for the Hibernate Search Standalone Pojo Mapper with Elasticsearch #39415, which must be merged first.=> DoneIt requires Rework how configuration doc generation works #35439 to get fixed first, and that will take a while; see https://quarkusio.zulipchat.com/#narrow/stream/187038-dev/topic/Reusing.20ConfigMapping.20from.20other.20JARS=> DoneBased on #43943, which must be merged first.=> Done