[GitHub] [camel-quarkus] davsclaus opened a new pull request #1526: CAMEL-14297: Introduce RouteBuilderConfigurer

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

[GitHub] [camel-quarkus] davsclaus opened a new pull request #1526: CAMEL-14297: Introduce RouteBuilderConfigurer

GitBox

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


   


----------------------------------------------------------------
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] davsclaus commented on pull request #1526: CAMEL-14297: Introduce RouteBuilderConfigurer

GitBox

davsclaus commented on pull request #1526:
URL: https://github.com/apache/camel-quarkus/pull/1526#issuecomment-672675824


   The test in
   org.apache.camel.quarkus.core.CoreTest#testLookupRoutes
   
   has a TODO as the 2nd route from RouteBuilderConfigurer is not discovered.
   
   It uses @Produces annotation from JEE but Camel cannot discover it. Not sure how we can make this possible.
   The regular RouteBuilder classes are discovered via jandex index and added during recorder magic.
   
   I would assume a @Produces annotation from CDI/JEE would also work. But since the bean is not injected somewhere then arc may not trigger it. So maybe we need some jandex magic to discover all methods that returns a RouteBuilderConfigurer and are annotated with @Produces should then record the method, or whatever needs to be done.


----------------------------------------------------------------
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 #1526: CAMEL-14297: Introduce RouteBuilderConfigurer

GitBox
In reply to this post by GitBox

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


   you can mark the bean as `@Unremovable` to avoid ArC to eliminate it, we can probably also add the `RouteBuilderConfigurer` class to the list of unmoveable beans


----------------------------------------------------------------
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 edited a comment on pull request #1526: CAMEL-14297: Introduce RouteBuilderConfigurer

GitBox
In reply to this post by GitBox

lburgazzoli edited a comment on pull request #1526:
URL: https://github.com/apache/camel-quarkus/pull/1526#issuecomment-672683300


   you can mark the bean as `@Unremovable` to avoid ArC to eliminate it, we can probably also add the `RouteBuilderConfigurer` class to the list of unmoveable beans: https://github.com/apache/camel-quarkus/blob/36c04ab508edac37a54705b3b0f48c1ab1a7d7a2/extensions-core/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/CamelProcessor.java#L95-L101


----------------------------------------------------------------
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] davsclaus commented on pull request #1526: CAMEL-14297: Introduce RouteBuilderConfigurer

GitBox
In reply to this post by GitBox

davsclaus commented on pull request #1526:
URL: https://github.com/apache/camel-quarkus/pull/1526#issuecomment-672686827


   Thanks @lburgazzoli that fixes 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] ppalaga commented on a change in pull request #1526: CAMEL-14297: Introduce RouteBuilderConfigurer

GitBox
In reply to this post by GitBox

ppalaga commented on a change in pull request #1526:
URL: https://github.com/apache/camel-quarkus/pull/1526#discussion_r469067913



##########
File path: integration-tests/core/src/test/java/org/apache/camel/quarkus/core/CoreTest.java
##########
@@ -33,12 +33,18 @@
 
 @QuarkusTest
 public class CoreTest {
+
     @Test
     public void testContainerLookupFromRegistry() {
         RestAssured.when().get("/test/registry/lookup-registry").then().body(is("true"));
         RestAssured.when().get("/test/registry/lookup-context").then().body(is("true"));
     }
 
+    @Test
+    public void testLookupRoutes() {
+        RestAssured.when().get("/test/routes/lookup-routes").then().body(is("true"));

Review comment:
       Comparing the list of routeIds here would make it easier to troubleshoot if something goes wrong.
   ```suggestion
           RestAssured.when().get("/test/routes/lookup-routes").then().body(is("foo,bar"));
   ```




----------------------------------------------------------------
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] davsclaus commented on a change in pull request #1526: CAMEL-14297: Introduce RouteBuilderConfigurer

GitBox
In reply to this post by GitBox

davsclaus commented on a change in pull request #1526:
URL: https://github.com/apache/camel-quarkus/pull/1526#discussion_r469074548



##########
File path: integration-tests/core/src/test/java/org/apache/camel/quarkus/core/CoreTest.java
##########
@@ -33,12 +33,18 @@
 
 @QuarkusTest
 public class CoreTest {
+
     @Test
     public void testContainerLookupFromRegistry() {
         RestAssured.when().get("/test/registry/lookup-registry").then().body(is("true"));
         RestAssured.when().get("/test/registry/lookup-context").then().body(is("true"));
     }
 
+    @Test
+    public void testLookupRoutes() {
+        RestAssured.when().get("/test/routes/lookup-routes").then().body(is("true"));

Review comment:
       Yeah good idea




----------------------------------------------------------------
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] ppalaga merged pull request #1526: CAMEL-14297: Introduce RouteBuilderConfigurer

GitBox
In reply to this post by GitBox

ppalaga merged pull request #1526:
URL: https://github.com/apache/camel-quarkus/pull/1526


   


----------------------------------------------------------------
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]