HSEARCH-5627 Switch from the Search session holder to a Hibernate ORM session extension#5137
Conversation
| else { | ||
| throw OrmMiscLog.INSTANCE.unsupportedSessionType( sessionImplementor.getClass() ); | ||
| } |
There was a problem hiding this comment.
Can you confirm that this code won't be reached when using a stateless session? I.e. we can still use a stateless session on a session factory that has Hibernate Search enabled, as long as we don't try to use Hibernate Search on the stateless session?
Or are we completely breaking stateless sessions for Hibernate Search users (beyond the simple fact that Hibernate Search was not effective in stateless sessions)?
There was a problem hiding this comment.
Hey 👋🏻 🙂
just pushed the tests with current behaviour:
- If someone tries to persist any indexed entities, we reach this point, and the operations will fail with a bit more meaningful message. We can actually now pass this point since the holder is gone and we can attach a search session to a stateless ORM one too, it's just that it won't work further down the road and fail with less informative message.
- if someone tries to persist any indexed entity but first disables the listener for "automatic" indexing -- things work ok
- if someone persists any non-indexed entities -- things are ok. (We have a check in the listener whether we need to get a session)
Sooo I think it's ok no? Maybe we can add the note to disable the listener if someone still wants to modify indexed entities in a stateless session ... but I'd rather the thing crash than pretend it synced the index while it didn't 🙈 🙂
There was a problem hiding this comment.
I'd rather the thing crash than pretend it synced the index while it didn't 🙈 🙂
Agreed
Maybe we can add the note to disable the listener if someone still wants to modify indexed entities in a stateless session
Agreed; I'd hope we have a section in the "limitations" section of the docs mentioning that stateless sessions are not supported? That would be the right place to document workarounds.
it's just that it won't work further down the road and fail with less informative message.
Er... that's a shame. Can we make it slightly better? I noticed we show the type we expect, but we don't show the actual session type encountered; maybe adding that would help?
marko-bekhta
force-pushed
the
feat/HSEARCH-5627-Switch-from-the-Search-session-holder-to-a-Hibernate-ORM-session-extension
branch
2 times, most recently
from
b14e2a9 to
42d979c
Compare
…extension # Conflicts: # build/config/pom.xml # build/parents/build/pom.xml
marko-bekhta
force-pushed
the
feat/HSEARCH-5627-Switch-from-the-Search-session-holder-to-a-Hibernate-ORM-session-extension
branch
from
42d979c to
7a6865e
Compare
|
2 participants
https://hibernate.atlassian.net/browse/HSEARCH-5627
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.