-
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
Update Reactive Messaging to version 2.2.0 and Integrate health support #10831
Update Reactive Messaging to version 2.2.0 and Integrate health support #10831
Conversation
Also fix the durability bug when sending messages to a non-durable AMQP address
bc66eb3
to
0dd61af
Compare
@ConfigRoot(name = "reactive-messaging", phase = ConfigPhase.BUILD_TIME) | ||
public class ReactiveMessagingBuildTimeConfig { | ||
/** | ||
* Whether or not an health check is published in case the smallrye-health extension is present. |
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 the in case...
part true? AFAICS, you always drag the smallrye-health
dependency.
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 only depends on the SPI, not on the extension itself.
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.
You sure? smallrye-reactive-messaging-health
doesn't depend on health?
Because if you don't have health, I'm surprised this can work: https://github.com/quarkusio/quarkus/pull/10831/files#diff-0e5b3cb2b027d18a3e49591822e8c947R310 . You would need to use a string for the class name as there's a good chance you wouldn't be able to load the checks if health is not present.
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.
smallrye-reactive-messaging-health depends on microprofile health not on the quarkus extension.
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.
https://github.com/quarkusio/quarkus/pull/10831/files#diff-0e5b3cb2b027d18a3e49591822e8c947R310 refers to classes from reactive messaging (in a specific module that depends on microprofile health).
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 just re-tested it does not pull quarkus-health, it's up to the user to use it.
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.
We discussed this with @cescoffier on Zulip. Let's merge as is even if not entirely consistent with the other SmallRye extensions.
No description provided.