-
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
Hibernate reactive panache refactoring #29761
Conversation
...mmon/runtime/src/main/java/io/quarkus/hibernate/reactive/panache/common/ReactiveSession.java
Outdated
Show resolved
Hide resolved
...me/src/main/java/io/quarkus/hibernate/reactive/panache/common/runtime/SessionOperations.java
Outdated
Show resolved
Hide resolved
edc1749
to
4b401be
Compare
e8b9bbe
to
50a181f
Compare
...mmon/runtime/src/main/java/io/quarkus/hibernate/reactive/panache/common/ReactiveSession.java
Outdated
Show resolved
Hide resolved
...rc/main/java/io/quarkus/hibernate/reactive/panache/common/runtime/ReactiveTransactional.java
Outdated
Show resolved
Hide resolved
...me/src/main/java/io/quarkus/hibernate/reactive/panache/common/runtime/SessionOperations.java
Show resolved
Hide resolved
...me/src/main/java/io/quarkus/hibernate/reactive/panache/common/runtime/SessionOperations.java
Outdated
Show resolved
Hide resolved
...me/src/main/java/io/quarkus/hibernate/reactive/panache/common/runtime/SessionOperations.java
Outdated
Show resolved
Hide resolved
...me/src/main/java/io/quarkus/hibernate/reactive/panache/common/runtime/SessionOperations.java
Show resolved
Hide resolved
...me/src/main/java/io/quarkus/hibernate/reactive/panache/common/runtime/SessionOperations.java
Show resolved
Hide resolved
...-reactive-panache/src/test/java/io/quarkus/it/panache/reactive/PanacheFunctionalityTest.java
Show resolved
Hide resolved
...active-panache/src/test/java/io/quarkus/it/panache/reactive/TestReactiveTransactionTest.java
Show resolved
Hide resolved
Sorry for the late review :( |
50a181f
to
db9a4ec
Compare
No problem, better late than never! ;-) |
db9a4ec
to
0b56ce8
Compare
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.
LGTM.
Do you think there's any need for a @WithoutSession
to disable the automatic @WithSessionIfNeeded
? I don't think so.
In Renarde, I automatically add the @Transactional
annotation to POST/PUT/DELETE
methods, but I've often wondered if I should add a @NoTransaction
annotation to disable this. Perhaps we need a @WithoutTransaction
in HR/Panache, so that I can avoid auto-adding @WithTransaction
when I integrate HR in Renarde?
I agree.
What would be the point of |
Because I automatically add the |
Well, I think that such an annotation should go into Renarde then... |
Yes, that's very possible. |
Hi @mkouba, not sure it is relevant, I was trying https://github.com/quarkusio/quarkus-quickstarts/tree/main/kafka-panache-reactive-quickstart (I composing a security demo from various quickstarts), and I've noticed, I believe, that if If it is indeed the case (confirmed) then may be it can make sense to have Sorry if it is off-topic |
I don't think it's required. I mean you just need to add an reactive SQL client extension like for any Hibernate Reactive application. |
But if without it NPE is returned then may be it is required ? |
Sorry, see what you mean, there are a few client types, may be a better error message can be returned like please add a client, etc |
I wonder if the NPE is thrown for vanilla Hibernate Reactive as well? |
I haven't tried, sorry, I just looked at Hibernate Panache Reactive as the code looks simpler which is what I'll need to for the introduction demo |
d804b2a
to
f7f931e
Compare
This comment has been minimized.
This comment has been minimized.
- do not store the current reactive session in the CDI request context but instead in the vertx duplicated context - do not offload execution of a panache entity method on the current vertx context but instead validate that the method is executed on the vetx duplicated context - introduce WithSession, WithSessionOnDemand and WithTransaction bindings and interceptors - deprecate ReactiveTransactional - ReactiveTransactionalInterceptor can only be used for methods that return Uni/Multi; this is validated at build time - if resteasy-reactive is present then automatically add WithSessionOnDemand binding to all resource methods on classes that use a panache entity - also remove the quarkus-integration-test-hibernate-reactive-panache-blocking module - quarkus-test-vertx - run the test method on a duplicated vertx context even if the RunOnVertxContext is not present but the TestReactiveTransaction is
- neither Hibernate Reactive nor reactive clients support streaming - furthermore, we're not able to provide a Panache#withTransaction() alternative for Multi without bypassing Hibernate Reactive API
f7f931e
to
99c4b70
Compare
✔️ The latest workflow run for the pull request has completed successfully. It should be safe to merge provided you have a look at the other checks in the summary. |
Thanks @mkouba |
+1 |
Disables messaging/qpid till quarkus-qpid-jms that uses Jakarta is released. Also disables external application tests till external projects (QS, Workshops, Todo app) migrates to Jakarta (that's till they use Quarkus 3+). Javax stuff is migrated to Jakarta and FW bumped to 1.3.0.Beta4 that also sports jakarta. Changes in Panache Hibernate Reactive module are forced by quarkusio/quarkus#29761
Disables messaging/qpid till quarkus-qpid-jms that uses Jakarta is released. Also disables external application tests till external projects (QS, Workshops, Todo app) migrates to Jakarta (that's till they use Quarkus 3+). Javax stuff is migrated to Jakarta and FW bumped to 1.3.0.Beta4 that also sports jakarta. Changes in Panache Hibernate Reactive module are forced by quarkusio/quarkus#29761
Disables messaging/qpid till quarkus-qpid-jms that uses Jakarta is released. Also disables external application tests till external projects (QS, Workshops, Todo app) migrates to Jakarta (that's till they use Quarkus 3+). Javax stuff is migrated to Jakarta and FW bumped to 1.3.0.Beta4 that also sports jakarta. Changes in Panache Hibernate Reactive module are forced by quarkusio/quarkus#29761
Disables messaging/qpid till quarkus-qpid-jms that uses Jakarta is released. Also disables external application tests till external projects (QS, Workshops, Todo app) migrates to Jakarta (that's till they use Quarkus 3+). Javax stuff is migrated to Jakarta and FW bumped to 1.3.0.Beta4 that also sports jakarta. Changes in Panache Hibernate Reactive module are forced by quarkusio/quarkus#29761
Disables messaging/qpid till quarkus-qpid-jms that uses Jakarta is released. Also disables external application tests till external projects (QS, Workshops, Todo app) migrates to Jakarta (that's till they use Quarkus 3+). Javax stuff is migrated to Jakarta and FW bumped to 1.3.0.Beta4 that also sports jakarta. Changes in Panache Hibernate Reactive module are forced by quarkusio/quarkus#29761
Disables messaging/qpid till quarkus-qpid-jms that uses Jakarta is released. Also disables external application tests till external projects (QS, Workshops, Todo app) migrates to Jakarta (that's till they use Quarkus 3+). Javax stuff is migrated to Jakarta and FW bumped to 1.3.0.Beta4 that also sports jakarta. Changes in Panache Hibernate Reactive module are forced by quarkusio/quarkus#29761
Disables messaging/qpid till quarkus-qpid-jms that uses Jakarta is released. Also disables external application tests till external projects (QS, Workshops, Todo app) migrates to Jakarta (that's till they use Quarkus 3+). Javax stuff is migrated to Jakarta and FW bumped to 1.3.0.Beta4 that also sports jakarta. Changes in Panache Hibernate Reactive module are forced by quarkusio/quarkus#29761
Disables messaging/qpid till quarkus-qpid-jms that uses Jakarta is released. Also disables external application tests till external projects (QS, Workshops, Todo app) migrates to Jakarta (that's till they use Quarkus 3+). Javax stuff is migrated to Jakarta and FW bumped to 1.3.0.Beta4 that also sports jakarta. Changes in Panache Hibernate Reactive module are forced by quarkusio/quarkus#29761
Disables messaging/qpid till quarkus-qpid-jms that uses Jakarta is released. Also disables external application tests till external projects (QS, Workshops, Todo app) migrates to Jakarta (that's till they use Quarkus 3+). Javax stuff is migrated to Jakarta and FW bumped to 1.3.0.Beta4 that also sports jakarta. Changes in Panache Hibernate Reactive module are forced by quarkusio/quarkus#29761
Disables messaging/qpid till quarkus-qpid-jms that uses Jakarta is released. Also disables external application tests till external projects (QS, Workshops, Todo app) migrates to Jakarta (that's till they use Quarkus 3+). Javax stuff is migrated to Jakarta and FW bumped to 1.3.0.Beta4 that also sports jakarta. Changes in Panache Hibernate Reactive module are forced by quarkusio/quarkus#29761
Disables messaging/qpid till quarkus-qpid-jms that uses Jakarta is released. Also disables external application tests till external projects (QS, Workshops, Todo app) migrates to Jakarta (that's till they use Quarkus 3+). Javax stuff is migrated to Jakarta and FW bumped to 1.3.0.Beta4 that also sports jakarta. Changes in Panache Hibernate Reactive module are forced by quarkusio/quarkus#29761
Disables messaging/qpid till quarkus-qpid-jms that uses Jakarta is released. Also disables external application tests till external projects (QS, Workshops, Todo app) migrates to Jakarta (that's till they use Quarkus 3+). Javax stuff is migrated to Jakarta and FW bumped to 1.3.0.Beta4 that also sports jakarta. Changes in Panache Hibernate Reactive module are forced by quarkusio/quarkus#29761
@WithSession
,@WithSessionOnDemand
and@WithTransaction
bindings and interceptorsReactiveTransactional
(WithTransaction
should be used instead)ReactiveTransactionalInterceptor
can only be used for methods that returnUni
; this is validated at build timeresteasy-reactive
is present then automatically add the@WithSessionOnDemand
binding to all resource methods on classes that use a panache entityquarkus-test-vertx
- run the test method on a duplicated vertx context even if the@RunOnVertxContext
is not present but the@TestReactiveTransaction
isintegration-tests/hibernate-reactive-panache-kotlin