-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
[WIP] Shared extension for the Hibernate Search Elasticsearch backend #39416
base: main
Are you sure you want to change the base?
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.
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.
This comment has been minimized.
This comment has been minimized.
44d8359
to
2c79e08
Compare
@gsmet I rebased my branch (this PR) on main and removed any reliance on my work in ##43808. You should be able to test your
Thanks for having a look! |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
In the light of the Spring config metadata, let's get rid of sourceClass and let's avoid using sourceType for the field/method type as it's confusing. Fixes quarkusio#43175
When we attach the config metadata, we need to consider only the properties of the current extension and not those from potential internal/common extensions: these will be published for the internal/common extensions themselves.
The all config guide might reference links from the current guide and we need to make sure they still work.
2c79e08
to
e5a7335
Compare
Status for workflow
|
🎊 PR Preview 5e4a69e has been successfully built and deployed to https://quarkus-pr-main-39416-preview.surge.sh/version/main/guides/
|
Status for workflow
|
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.