You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2021/03/16 09:03:49 UTC

[GitHub] [camel-quarkus] jamesnetherton opened a new pull request #2338: Add configuration option to ignore unknown arguments

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


   I set the default behaviour to throw an exception if unknown args are found.
   
   Maybe the default should be to log a warning instead?


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [camel-quarkus] jamesnetherton commented on a change in pull request #2338: Add configuration option to ignore unknown arguments

Posted by GitBox <gi...@apache.org>.
jamesnetherton commented on a change in pull request #2338:
URL: https://github.com/apache/camel-quarkus/pull/2338#discussion_r595087911



##########
File path: extensions-core/main/runtime/src/main/java/org/apache/camel/quarkus/main/CamelMainConfig.java
##########
@@ -40,4 +46,27 @@
         @ConfigItem(defaultValue = "PT3S")
         public Duration timeout;
     }
+
+    @ConfigGroup
+    public static class ArgumentConfig {
+
+        /**
+         * The action to take when {@link CamelMain} encounters an unknown argument
+         */
+        @ConfigItem(defaultValue = "FAIL")
+        public UnknownArgumentFailMode onUnknown;
+    }
+
+    /**
+     * Options defining the action to take in response to errors parsing unknown command line arguments
+     *
+     * FAIL - Prints the {@link CamelMain} usage statement and throws a {@link RuntimeException}
+     * IGNORE - Suppresses any warnings and the application startup proceeds as normal
+     * WARN - Prints the {@link CamelMain} usage statement but allows the application startup to proceed as normal
+     */
+    public enum UnknownArgumentFailMode {
+        FAIL,
+        IGNORE,
+        WARN
+    }

Review comment:
       Latest commit applies those suggestions.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [camel-quarkus] jamesnetherton commented on pull request #2338: Add configuration option to ignore unknown arguments

Posted by GitBox <gi...@apache.org>.
jamesnetherton commented on pull request #2338:
URL: https://github.com/apache/camel-quarkus/pull/2338#issuecomment-800201697


   Latest commit changes the default to `warn`.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [camel-quarkus] jamesnetherton merged pull request #2338: Add configuration option to ignore unknown arguments

Posted by GitBox <gi...@apache.org>.
jamesnetherton merged pull request #2338:
URL: https://github.com/apache/camel-quarkus/pull/2338


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [camel-quarkus] jamesnetherton commented on pull request #2338: Add configuration option to ignore unknown arguments

Posted by GitBox <gi...@apache.org>.
jamesnetherton commented on pull request #2338:
URL: https://github.com/apache/camel-quarkus/pull/2338#issuecomment-800192596


   > I am not going to veto, if Luca favors that :)
   
   I think it's actually my preference too. The warning is big and ugly enough that folks should hopefully notice something is wrong.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [camel-quarkus] lburgazzoli commented on pull request #2338: Add configuration option to ignore unknown arguments

Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on pull request #2338:
URL: https://github.com/apache/camel-quarkus/pull/2338#issuecomment-800192706


   Ah, I did it from the mobile app and it was mainly for the PR per se not specifically on the warn vs exception so @ppalaga you can put your veto :)


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [camel-quarkus] lburgazzoli edited a comment on pull request #2338: Add configuration option to ignore unknown arguments

Posted by GitBox <gi...@apache.org>.
lburgazzoli edited a comment on pull request #2338:
URL: https://github.com/apache/camel-quarkus/pull/2338#issuecomment-800192706


   Ah, I did it from the mobile app and it was mainly for the PR per se not specifically on the warn vs exception so @ppalaga you can put your veto :) but still my preference is for a warning


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [camel-quarkus] ppalaga commented on pull request #2338: Add configuration option to ignore unknown arguments

Posted by GitBox <gi...@apache.org>.
ppalaga commented on pull request #2338:
URL: https://github.com/apache/camel-quarkus/pull/2338#issuecomment-800191753


   > should be to log a warning instead?
   
   I am not going to veto, if Luca favors 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:
users@infra.apache.org



[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #2338: Add configuration option to ignore unknown arguments

Posted by GitBox <gi...@apache.org>.
ppalaga commented on a change in pull request #2338:
URL: https://github.com/apache/camel-quarkus/pull/2338#discussion_r595077840



##########
File path: extensions-core/main/runtime/src/main/java/org/apache/camel/quarkus/main/CamelMainConfig.java
##########
@@ -40,4 +46,27 @@
         @ConfigItem(defaultValue = "PT3S")
         public Duration timeout;
     }
+
+    @ConfigGroup
+    public static class ArgumentConfig {
+
+        /**
+         * The action to take when {@link CamelMain} encounters an unknown argument
+         */
+        @ConfigItem(defaultValue = "FAIL")
+        public UnknownArgumentFailMode onUnknown;
+    }
+
+    /**
+     * Options defining the action to take in response to errors parsing unknown command line arguments
+     *
+     * FAIL - Prints the {@link CamelMain} usage statement and throws a {@link RuntimeException}
+     * IGNORE - Suppresses any warnings and the application startup proceeds as normal
+     * WARN - Prints the {@link CamelMain} usage statement but allows the application startup to proceed as normal
+     */
+    public enum UnknownArgumentFailMode {
+        FAIL,
+        IGNORE,
+        WARN
+    }

Review comment:
       There is `org.apache.camel.quarkus.core.CamelConfig.FailureRemedy` which I assume is visible here. Shouldn't we re-use it?  
   Wouldn't the `ArgumentConfig.onUnknown` field be a better place for the FAIL/IGNORE/WARN JavaDoc? - in that way user could see it on the extension page. 




----------------------------------------------------------------
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:
users@infra.apache.org