Tries to inject Vertx instance from CDI using a qualifier #2772
Conversation
...vider/src/main/java/io/smallrye/reactive/messaging/providers/connectors/ExecutionHolder.java
Outdated
Show resolved
Hide resolved
...vider/src/main/java/io/smallrye/reactive/messaging/providers/connectors/ExecutionHolder.java
Outdated
Show resolved
Hide resolved
| @@ -19,6 +22,9 @@ | |||
| @ApplicationScoped | |||
| public class ExecutionHolder { | |||
|
|
|||
| private static final String REACTIVE_MESSAGING_VERTX_CDI_QUALIFIER = "mp.messaging.connector.vertx.cdi.qualifier"; | |||
| private static final String REACTIVE_MESSAGING_VERTX_CDI_QUALIFIER_REQUIRED = "mp.messaging.connector.vertx.cdi.qualifier.required"; | |||
There was a problem hiding this comment.
Do we really need a second property ?
Can't we just look for "mp.messaging.connector.vertx.identifier" and if it is defined look up the instance with it?
There was a problem hiding this comment.
+1, just the identifier value should be fine.
There was a problem hiding this comment.
Maybe not using Identifier, I will test to confirm that. The case it tries to cover is when there is a vertx instance exposed with a cdi qualifier(Named("vertx")) but the Instance<Vertx> ist in the ExecutionHolder, the ist.isUnsatisfied() is false, which forces sr rm to inject the instance.
BTW, I will be on holidays this week, will report back next week, thanks :)
There was a problem hiding this comment.
Done by using only the first option and the @io.smallrye.common.annotation.Identifier qualifier only.
The vertx bean from the vertx subsystem is exposed with 2 qualifiers: @Any and @Identifier, so to select the instance only when the cdi qualifier is specified, I split the @Default out in the else check block.
|
Thank you all for the reviews, I updated the PR, would you please review again ? thank you :) |
|
@cescoffier @ozangunalp would you please review again ? thanks :) |
...vider/src/main/java/io/smallrye/reactive/messaging/providers/connectors/ExecutionHolder.java
Outdated
Show resolved
Hide resolved
|
@ozangunalp thank you for the reviews, I updated to add a test in the provider module, please let me know if there is anything needs to be fixed or improved. :) |
|
@cescoffier @ozangunalp would you please review again ? thanks |
|
@gaol looks good to me, I'll let @cescoffier to have another look. |
| @@ -147,4 +147,8 @@ public interface ProviderLogging extends BasicLogger { | |||
| @LogMessage(level = Logger.Level.WARN) | |||
| @Message(id = 243, value = "Processing method '%s' annotated with @Acknowledgement(POST_PROCESSING), but may not be compatible with post-processing acknowledgement management. You may experience duplicate (negative-)acknowledgement of messages.") | |||
| void postProcessingNotFullySupported(String methodAsString); | |||
|
|
|||
| @LogMessage(level = Logger.Level.INFO) | |||
There was a problem hiding this comment.
Should be debug - not very informative for regular users
|
Thanks @ozangunalp and @cescoffier , updated again to change log level to DEBUG. :) |
cescoffier
changed the title
Fixes #2771
This PR tries to add 2 config items:
mp.messaging.connector.vertx.cdi.qualifier, which is used to tries to inject the Vertx instance with the specified qualifier if configured.mp.messaging.connector.vertx.cdi.qualifier.required, boolean, which is used to indicate if the qualifier is required in case it comes from CDI, otherwise it will create a new Vertx instance like what current does.ExecutionHolderis used by both WildFly and Quarkus. And for WildFly, there is a coming vertx subsystem to be able to configure the VertxOptions following the WildFly management model style, which will expose a CDI bean with a qualifier so that other subsystems can select it by themselves. The 2nd config item can be included in the integration code of WildFly so that applications deployed to WildFly will inject only the instance with a qualifier, and leave the use case in Quarkus not changed at all.