[GitHub] [camel-quarkus] squakez opened a new pull request #1688: Feat(mongodb): add support for named client

classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] squakez opened a new pull request #1688: Feat(mongodb): add support for named client

GitBox

squakez opened a new pull request #1688:
URL: https://github.com/apache/camel-quarkus/pull/1688


   As referenced in #1608 we were not yet supporting the possibility to inject named client connection. With a fix provided to quarkus extension and this work PR we will be able to inject a "named" client that can be configurable in `application.properties` and referenced in route with its name.
   
   Closes #1608


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #1688: Feat(mongodb): add support for named client

GitBox

lburgazzoli commented on a change in pull request #1688:
URL: https://github.com/apache/camel-quarkus/pull/1688#discussion_r483507020



##########
File path: extensions-support/mongodb/deployment/src/main/java/org/apache/camel/quarkus/support/mongodb/deployment/SupportMongoDBProcessor.java
##########
@@ -35,20 +35,23 @@ FeatureBuildItem feature() {
     }
 
     @BuildStep
-    void registerCamelMongoClientProducer(
+    void registerCamelMongoClientProducers(
             List<MongoClientBuildItem> mongoClients,
             BuildProducer<CamelRuntimeBeanBuildItem> runtimeBeans) {
 
         for (MongoClientBuildItem mongoClient : mongoClients) {
-            // If there is a default mongo client instance, then bind it to the camel registry
-            // with the default mongo client name used by the camel-mongodb component
-            if (MongoClientBeanUtil.isDefault(mongoClient.getName())) {
-                runtimeBeans.produce(
-                        new CamelRuntimeBeanBuildItem(
-                                "camelMongoClient",
-                                "com.mongodb.client.MongoClient",
-                                mongoClient.getClient()));
-            }
+            String clientName = getMongoClientName(mongoClient.getName());

Review comment:
       isn't quarkus already publishing the MongoClients to CDI ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] squakez commented on pull request #1688: Feat(mongodb): add support for named client

GitBox
In reply to this post by GitBox

squakez commented on pull request #1688:
URL: https://github.com/apache/camel-quarkus/pull/1688#issuecomment-687061759


   Thanks for the review. I am not sure of how to proceed further as I've just started working with quarkus extensions. I see [this method](https://github.com/quarkusio/quarkus/blob/master/extensions/mongodb-client/deployment/src/main/java/io/quarkus/mongodb/deployment/MongoClientProcessor.java#L288-L306) in the `quarkus` extension that we are later using in this camel extension to bind a client to the registry.
   
   I've played a bit removing the whole `@BuildStep` on the extension resulting in a `No bean could be found in the registry for: mongoClient1 of type: com.mongodb.client.MongoClient` exception. Please @oscerd, @lburgazzoli any hint or example on how to proceed would help me a lot :)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on pull request #1688: Feat(mongodb): add support for named client

GitBox
In reply to this post by GitBox

lburgazzoli commented on pull request #1688:
URL: https://github.com/apache/camel-quarkus/pull/1688#issuecomment-687066865


   we need to check on the quarkus side if such beans are also published and if they are why the don't get resolved.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] squakez commented on pull request #1688: Feat(mongodb): add support for named client

GitBox
In reply to this post by GitBox

squakez commented on pull request #1688:
URL: https://github.com/apache/camel-quarkus/pull/1688#issuecomment-688690644


   I've made a few tests and according to what I've managed to see there is no client bean into CDI, unless that `MongoClientBuildItem` is used (either for a default or named connection). I've opened a thread to figure out if that is the expected behavior and why: https://quarkusio.zulipchat.com/#narrow/stream/187030-users/topic/MongoDB.20clients.20CDI


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] squakez commented on pull request #1688: Feat(mongodb): add support for named client

GitBox
In reply to this post by GitBox

squakez commented on pull request #1688:
URL: https://github.com/apache/camel-quarkus/pull/1688#issuecomment-690276425


   As we're discussing with Quarkus community, it seems the extension is not putting the CDI names to the clients, reason why we don't automatically bind them to the registry.
   
   There is an [opened PR to fix that issue](https://github.com/quarkusio/quarkus/pull/11998), but it opened a [discussion](https://quarkusio.zulipchat.com/#narrow/stream/187030-users/topic/MongoDB.20clients.20CDI) that is still ongoing and probably won't be resolved in short term.
   
   As the work I've done is not really adding any further dependency, I propose to follow up with this development for now as it adds the possibility to use a named client on the mongo component within a method that is already called on our side. I will open a follow up request on our side in order to track the progress on the Quarkus community and refine this component once they will introduce those required adjustments.
   
   @oscerd, @lburgazzoli what do you think?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on pull request #1688: Feat(mongodb): add support for named client

GitBox
In reply to this post by GitBox

lburgazzoli commented on pull request #1688:
URL: https://github.com/apache/camel-quarkus/pull/1688#issuecomment-690282592


   @squakez fine with me


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] squakez commented on pull request #1688: Feat(mongodb): add support for named client

GitBox
In reply to this post by GitBox

squakez commented on pull request #1688:
URL: https://github.com/apache/camel-quarkus/pull/1688#issuecomment-693283045


   @oscerd there is still pending a change requested on your side. As I wrote in previous comment this is actually the only way to go ahead at the moment, can you please have a second look and confirm if you have any objection on merging this? Thanks!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] oscerd commented on pull request #1688: Feat(mongodb): add support for named client

GitBox
In reply to this post by GitBox

oscerd commented on pull request #1688:
URL: https://github.com/apache/camel-quarkus/pull/1688#issuecomment-693289524


   Sorry, I forgot about this PR


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] squakez commented on pull request #1688: Feat(mongodb): add support for named client

GitBox
In reply to this post by GitBox

squakez commented on pull request #1688:
URL: https://github.com/apache/camel-quarkus/pull/1688#issuecomment-693292956


   > Sorry, I forgot about this PR
   
   No problem, thanks for reviewing it!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] jamesnetherton commented on pull request #1688: Feat(mongodb): add support for named client

GitBox
In reply to this post by GitBox

jamesnetherton commented on pull request #1688:
URL: https://github.com/apache/camel-quarkus/pull/1688#issuecomment-693346358


   This looks ok to me. We should add a test for the named client.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] squakez commented on pull request #1688: Feat(mongodb): add support for named client

GitBox
In reply to this post by GitBox

squakez commented on pull request #1688:
URL: https://github.com/apache/camel-quarkus/pull/1688#issuecomment-693383872


   > This looks ok to me. We should add a test for the named client.
   
   I can have a look at that.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] squakez commented on pull request #1688: Feat(mongodb): add support for named client

GitBox
In reply to this post by GitBox

squakez commented on pull request #1688:
URL: https://github.com/apache/camel-quarkus/pull/1688#issuecomment-696642955


   @jamesnetherton I've added a suite of test to cover also the named clients.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] jamesnetherton commented on pull request #1688: Feat(mongodb): add support for named client

GitBox
In reply to this post by GitBox

jamesnetherton commented on pull request #1688:
URL: https://github.com/apache/camel-quarkus/pull/1688#issuecomment-696645126


   Thanks @squakez!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] jamesnetherton merged pull request #1688: Feat(mongodb): add support for named client

GitBox
In reply to this post by GitBox

jamesnetherton merged pull request #1688:
URL: https://github.com/apache/camel-quarkus/pull/1688


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] jamesnetherton commented on pull request #1688: Feat(mongodb): add support for named client

GitBox
In reply to this post by GitBox

jamesnetherton commented on pull request #1688:
URL: https://github.com/apache/camel-quarkus/pull/1688#issuecomment-696645126


   Thanks @squakez!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] jamesnetherton merged pull request #1688: Feat(mongodb): add support for named client

GitBox
In reply to this post by GitBox

jamesnetherton merged pull request #1688:
URL: https://github.com/apache/camel-quarkus/pull/1688


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] squakez commented on pull request #1688: Feat(mongodb): add support for named client

GitBox
In reply to this post by GitBox

squakez commented on pull request #1688:
URL: https://github.com/apache/camel-quarkus/pull/1688#issuecomment-696642955


   @jamesnetherton I've added a suite of test to cover also the named clients.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]