-
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
Upgrade to Hibernate Search 6.0.0.Beta9 + Elasticsearch 7.8 #11166
Conversation
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 added some question/suggestion. The first one is a real question: I'm not sure I understand why the default backend now needs to be disabled?
// There is at least one named backend: we disallow using the default backend. | ||
// Theoretically we could allow using it, | ||
// but then we would be unable to detect whether the default backend is used | ||
// (and thus absolutely needs the version to be configured) |
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 sure I follow you here. We make the version mandatory for named backend so maybe we should just make it mandatory for everything, default backend included? And then we could keep the named backend active?
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.
If we do that, then people using named backends exclusively (which, IMO, is what they should do if they have multiple backends) will have to set the version for the default backend even if they don't use it. I can do that if you want, but I'm not sure it's a better solution.
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 fixed this according to our discussion on Zulip, rebased and force-pushed. Let's wait for CI, but then as far as I know it can be merged.
...uarkus/hibernate/search/elasticsearch/runtime/HibernateSearchElasticsearchRuntimeConfig.java
Outdated
Show resolved
Hide resolved
I had a quick look at the build failure, and it seems unrelated. I suspect a problem on master? |
@gsmet Also, for the migration guide:
|
@gsmet should I do anything or is it ok to merge ? |
… instead of defining our own
This feature probably wasn't used a lot by Quarkus users, since it's only (potentially) useful when you have multiple backends.
…fault configuration for indexes
…ifying default configuration for indexes
I had renamed the tests in 961d95b because I misunderstood their purpose from the comment on IndexedEntity and thought they were misnamed, but actually it's the comment that was unclear and should have been fixed.
d908226
to
2e6f816
Compare
Relevant: https://in.relation.to/2020/08/03/hibernate-search-6-0-0-Beta9/