[GitHub] [camel-quarkus] lburgazzoli opened a new pull request #704: Ensure that catalog files are added to the native image

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

[GitHub] [camel-quarkus] lburgazzoli opened a new pull request #704: Ensure that catalog files are added to the native image

GitBox
lburgazzoli opened a new pull request #704: Ensure that catalog files are added to the native image
URL: https://github.com/apache/camel-quarkus/pull/704
 
 
   Fixes #686

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


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #704: Ensure that catalog files are added to the native image

GitBox
ppalaga commented on a change in pull request #704: Ensure that catalog files are added to the native image
URL: https://github.com/apache/camel-quarkus/pull/704#discussion_r378123185
 
 

 ##########
 File path: extensions/core/runtime/src/main/java/org/apache/camel/quarkus/core/CamelConfig.java
 ##########
 @@ -174,4 +180,48 @@
         public Optional<List<String>> includePatterns;
     }
 
+    @ConfigGroup
+    public static class RuntimeCatalogConfig {
+        /**
+         * Enable {@link CamelRuntimeCatalog} functionaries.
+         */
+        @ConfigItem(defaultValue = "true")
+        public boolean enabled;
+
+        /**
+         * Used to control the resolution of components catalog info.
 
 Review comment:
   Is "resolution" really what this does? Would not "inclusion" suit better?
   
   Again, it may be unclear to the reader, what is catalog. Maybe a link to a Camel docs page explaining the concept would be nice. The description should give me a hint why/when should I care, why would I ever need this to be true?
   
   This also applies to all other options in this file.
   
   

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


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #704: Ensure that catalog files are added to the native image

GitBox
In reply to this post by GitBox
ppalaga commented on a change in pull request #704: Ensure that catalog files are added to the native image
URL: https://github.com/apache/camel-quarkus/pull/704#discussion_r378114899
 
 

 ##########
 File path: extensions/core/runtime/src/main/java/org/apache/camel/quarkus/core/CamelConfig.java
 ##########
 @@ -174,4 +180,48 @@
         public Optional<List<String>> includePatterns;
     }
 
+    @ConfigGroup
+    public static class RuntimeCatalogConfig {
+        /**
+         * Enable {@link CamelRuntimeCatalog} functionaries.
 
 Review comment:
   I have just learned about the existence of `functionary/functionaries` as a valid work of the English language. Anyway: is that not a typo?
   
   In the description of the option, I'd like to learn what exactly happens when the option is true and what happens otherwise.
   
   If I am a new user, I may have no idea what is CamelRuntimeCatalog. Yes there is a link to CamelRuntimeCatalog, but that class has no JavaDoc. Moreover, the link will most probably not work everywhere (IDEs, Web docs) where the text will be displayed. So either a working link to a resource explaining the concept of the catalog or a short inline definition would be nice.
   
   It would also be nice to document the relationship (if there is any) to other options in this file - e.g. does enabled = false imply that all other options in this file are ignored?
   
   Does this option only matter for the native mode? What happens in the JVM mode then?

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


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #704: Ensure that catalog files are added to the native image

GitBox
In reply to this post by GitBox
lburgazzoli commented on a change in pull request #704: Ensure that catalog files are added to the native image
URL: https://github.com/apache/camel-quarkus/pull/704#discussion_r378130392
 
 

 ##########
 File path: extensions/core/runtime/src/main/java/org/apache/camel/quarkus/core/CamelConfig.java
 ##########
 @@ -174,4 +180,48 @@
         public Optional<List<String>> includePatterns;
     }
 
+    @ConfigGroup
+    public static class RuntimeCatalogConfig {
+        /**
+         * Enable {@link CamelRuntimeCatalog} functionaries.
+         */
+        @ConfigItem(defaultValue = "true")
+        public boolean enabled;
+
+        /**
+         * Used to control the resolution of components catalog info.
 
 Review comment:
   > Is "resolution" really what this does? Would not "inclusion" suit better?
   >
   
   
   It is resolution, if you go through the code you'd see that this flag controls if the runtime catalog should attempt or not to resolve the given name to its model
   
   
   > Again, it may be unclear to the reader, what is catalog. Maybe a link to a Camel docs page explaining the concept would be nice. The description should give me a hint why/when should I care, why would I ever need this to be true?
   >
   
   The impact on the native image is described a few lines below.
   

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


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #704: Ensure that catalog files are added to the native image

GitBox
In reply to this post by GitBox
lburgazzoli commented on a change in pull request #704: Ensure that catalog files are added to the native image
URL: https://github.com/apache/camel-quarkus/pull/704#discussion_r378132197
 
 

 ##########
 File path: extensions/core/runtime/src/main/java/org/apache/camel/quarkus/core/CamelConfig.java
 ##########
 @@ -174,4 +180,48 @@
         public Optional<List<String>> includePatterns;
     }
 
+    @ConfigGroup
+    public static class RuntimeCatalogConfig {
+        /**
+         * Enable {@link CamelRuntimeCatalog} functionaries.
 
 Review comment:
   > I have just learned about the existence of `functionary/functionaries` as a valid work of the English language. Anyway: is that not a typo?
   >
   > In the description of the option, I'd like to learn what exactly happens when the option is true and what happens otherwise.
   >
   
   It is a typo
   
   > If I am a new user, I may have no idea what is CamelRuntimeCatalog. Yes there is a link to CamelRuntimeCatalog, but that class has no JavaDoc. Moreover, the link will most probably not work everywhere (IDEs, Web docs) where the text will be displayed. So either a working link to a resource explaining the concept of the catalog or a short inline definition would be nice.
   >
   
   I don't think we need to describe what the RuntimeCamelContext is in camel-quarkus so if there's lack of documentation on camel side, please raise an issue there.
   
   > It would also be nice to document the relationship (if there is any) to other options in this file - e.g. does enabled = false imply that all other options in this file are ignored?
   >
   > Does this option only matter for the native mode? What happens in the JVM mode then?
   
   Agree, this need to be improved
   

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


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #704: Ensure that catalog files are added to the native image

GitBox
In reply to this post by GitBox
lburgazzoli commented on a change in pull request #704: Ensure that catalog files are added to the native image
URL: https://github.com/apache/camel-quarkus/pull/704#discussion_r378143829
 
 

 ##########
 File path: extensions/core/runtime/src/main/java/org/apache/camel/quarkus/core/CamelConfig.java
 ##########
 @@ -174,4 +180,48 @@
         public Optional<List<String>> includePatterns;
     }
 
+    @ConfigGroup
+    public static class RuntimeCatalogConfig {
+        /**
+         * Enable {@link CamelRuntimeCatalog} functionaries.
 
 Review comment:
   should be fixed now

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


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #704: Ensure that catalog files are added to the native image

GitBox
In reply to this post by GitBox
lburgazzoli commented on a change in pull request #704: Ensure that catalog files are added to the native image
URL: https://github.com/apache/camel-quarkus/pull/704#discussion_r378143880
 
 

 ##########
 File path: extensions/core/runtime/src/main/java/org/apache/camel/quarkus/core/CamelConfig.java
 ##########
 @@ -174,4 +180,48 @@
         public Optional<List<String>> includePatterns;
     }
 
+    @ConfigGroup
+    public static class RuntimeCatalogConfig {
+        /**
+         * Enable {@link CamelRuntimeCatalog} functionaries.
+         */
+        @ConfigItem(defaultValue = "true")
+        public boolean enabled;
+
+        /**
+         * Used to control the resolution of components catalog info.
 
 Review comment:
   should be fixed now

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


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on issue #704: Ensure that catalog files are added to the native image

GitBox
In reply to this post by GitBox
ppalaga commented on issue #704: Ensure that catalog files are added to the native image
URL: https://github.com/apache/camel-quarkus/pull/704#issuecomment-585134697
 
 
   Thanks for the improvements. I still think the config options we expose should be documented so that they make sense to a newcomer. If there is no Camel page explaining the nature and purpose of the catalog, then I vote for adding a sentence here rather than waiting for it to happen elsewhere. I'd be ready to write that sentence but I am quite frankly missing the required knowledge.

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


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on issue #704: Ensure that catalog files are added to the native image

GitBox
In reply to this post by GitBox
lburgazzoli commented on issue #704: Ensure that catalog files are added to the native image
URL: https://github.com/apache/camel-quarkus/pull/704#issuecomment-585137226
 
 
   > Thanks for the improvements. I still think the config options we expose should be documented so that they make sense to a newcomer. If there is no Camel page explaining the nature and purpose of the catalog.
   
   Please rise an issue on camel side, we should improve camel in general rather than this specific sub-project so we can do the work only once for everyone to benefit.
   
   btw, there are already some javadocs - maybe limited - on the related classes on camel side:
   - https://github.com/apache/camel/blob/master/catalog/camel-catalog/src/main/java/org/apache/camel/catalog/CamelCatalog.java
   - https://github.com/apache/camel/blob/master/core/camel-api/src/main/java/org/apache/camel/catalog/RuntimeCamelCatalog.java
   
   
   

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


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli edited a comment on issue #704: Ensure that catalog files are added to the native image

GitBox
In reply to this post by GitBox
lburgazzoli edited a comment on issue #704: Ensure that catalog files are added to the native image
URL: https://github.com/apache/camel-quarkus/pull/704#issuecomment-585137226
 
 
   > Thanks for the improvements. I still think the config options we expose should be documented so that they make sense to a newcomer. If there is no Camel page explaining the nature and purpose of the catalog.
   
   Please raise an issue on camel side, we should improve camel in general rather than this specific sub-project so we can do the work only once for everyone to benefit.
   
   btw, there are already some javadocs - maybe limited - on the related classes on camel side:
   - https://github.com/apache/camel/blob/master/catalog/camel-catalog/src/main/java/org/apache/camel/catalog/CamelCatalog.java
   - https://github.com/apache/camel/blob/master/core/camel-api/src/main/java/org/apache/camel/catalog/RuntimeCamelCatalog.java
   
   
   

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


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on issue #704: Ensure that catalog files are added to the native image

GitBox
In reply to this post by GitBox
ppalaga commented on issue #704: Ensure that catalog files are added to the native image
URL: https://github.com/apache/camel-quarkus/pull/704#issuecomment-585406473
 
 
   Done https://github.com/apache/camel-quarkus/issues/706 and https://issues.apache.org/jira/browse/CAMEL-14545

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


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli merged pull request #704: Ensure that catalog files are added to the native image

GitBox
In reply to this post by GitBox
lburgazzoli merged pull request #704: Ensure that catalog files are added to the native image
URL: https://github.com/apache/camel-quarkus/pull/704
 
 
   

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


With regards,
Apache Git Services