-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Panache and Hibernate Reactive use different Vert.x local contexts #47441
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
base: main
Are you sure you want to change the base?
Conversation
@DavideD it seems there are unrelated changes in this PR, correct? |
|
Nevermind, I rebased it and now this is the commit with the important changes: 3093299 |
...me/src/main/java/io/quarkus/hibernate/reactive/panache/common/runtime/SessionOperations.java
Outdated
Show resolved
Hide resolved
In preparation to the upgrade to Vertx. 5, Hibernate Reactive doesn't use anymore the deprecated API to access the local context. This commits updates the way Panache access the context so that the two extensions can comunicate. Fix quarkusio#47314 Relats to the Hibernate Reactive issues: * hibernate/hibernate-reactive#2176 * hibernate/hibernate-reactive#2174 * hibernate/hibernate-reactive#2207
@tsegismont I moved the method to access the map in ContextualDataStorage, and updated this PR. Would this work? Do you think I also need to udpate the following code in VertxContextSafety? I don't think so but I'm not sure. |
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.
It looks good to me, thank you @DavideD
We could avoid to retrieve the Vert.x context twice, is that what you were thinking about? |
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.
Ah, I suppose this will require a similar fix in my Panache 2 branch, due to me adding stateless sessions support in the same area.
Possible fix for #47314
Draft because it needs these changes in Hibernate Reactive (next release).
@tsegismont, or @vietj, Do these changes make sense?
Thanks