-
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
Allow injection of SearchSession/SearchMapping #13696
Conversation
a070752
to
ba58b16
Compare
ba58b16
to
b7dd16e
Compare
<plugin> | ||
<artifactId>maven-surefire-plugin</artifactId> | ||
<configuration> | ||
<skip>true</skip> |
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.
Is this intended?
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.
Yes, they need Elasticsearch so they are run if -Dtest-containers
quarkus.hibernate-search-orm.elasticsearch.hosts=${elasticsearch.hosts:localhost:9200} | ||
quarkus.hibernate-search-orm.elasticsearch.protocol=${elasticsearch.protocol:http} | ||
quarkus.hibernate-search-orm.schema-management.strategy=drop-and-create-and-drop | ||
quarkus.hibernate-search-orm.automatic-indexing.synchronization.strategy=sync |
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.
Don't you expect the configuration to be potentially different depending on the PU?
Doesn't need to be part of this PR, just asking.
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.
Right now you can make do by defining one backend per PU, but some settings (like this one) will have to be the same on all PUs, which is not great.
So yes, we'd need additional changes to fully support multiple PUs. In another PR :)
…ts API contracts This is important for Hibernate Search in particular.
…y from a TransactionScopedSession This is important for Hibernate Search in particular. Signed-off-by: Yoann Rodière <[email protected]>
Signed-off-by: Yoann Rodière <[email protected]>
Signed-off-by: Yoann Rodière <[email protected]>
…orm-elasticsearch/deployment Signed-off-by: Yoann Rodière <[email protected]>
Signed-off-by: Yoann Rodière <[email protected]>
Signed-off-by: Yoann Rodière <[email protected]>
Signed-off-by: Yoann Rodière <[email protected]>
b7dd16e
to
abf99e7
Compare
This extension is based on a beta version of Hibernate Search. | ||
While APIs are quite stable and the code is of production quality and thoroughly tested, some features are still missing, performance might not be optimal and some APIs or configuration properties might change as the extension matures. | ||
==== | ||
|
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.
💯
---- | ||
<1> Inject a Hibernate Search session, which relies on the `EntityManager` under the hood. | ||
Applications with multiple persistence units can use a CDI qualifier to select the right one: | ||
either `@Named` or `@io.quarkus.hibernate.orm.PersistenceUnit`. |
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.
Not a blocker, but I'd prefer such a note to be fully expressed with an example.
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.
Ok, I'll add an example in the next PR that adds first-class support for multiple PUs.
Based on #13694, which should be merged first. Creating this PR as a draft until it's done.Related quickstart update: quarkusio/quarkus-quickstarts#725