mimaison commented on code in PR #15457:
URL: https://github.com/apache/kafka/pull/15457#discussion_r1514959532


##########
tools/src/main/java/org/apache/kafka/tools/consumer/ConsoleConsumerOptions.java:
##########
@@ -322,6 +324,20 @@ private void invalidOffset(String offset) {
                 "'earliest', 'latest', or a non-negative long.");
     }
 
+    private long parseTimeoutMs() {
+        long timeout;
+        if (options.has(timeoutMsOpt)) {
+            timeout = options.valueOf(timeoutMsOpt);
+            if (timeout < 0) {
+                CommandLineUtils.printUsageAndExit(parser, "The provided 
timeout-ms value '" + timeout +

Review Comment:
   It seems previously we wouldn't fail if a negative value was provided.



##########
tools/src/main/java/org/apache/kafka/tools/consumer/ConsoleConsumerOptions.java:
##########
@@ -385,10 +404,14 @@ String bootstrapServer() {
         return options.valueOf(bootstrapServerOpt);
     }
 
-    String includedTopicsArg() {
-        return options.has(includeOpt)
-                ? options.valueOf(includeOpt)
-                : options.valueOf(whitelistOpt);
+    Optional<String> includedTopicsArg() {
+        if (options.has(includeOpt)) {
+            return Optional.of(options.valueOf(includeOpt));
+        } else if (options.has(whitelistOpt)) {
+            return Optional.of(options.valueOf(whitelistOpt));
+        } else {
+            return Optional.empty();
+        }

Review Comment:
   Could this be simplified into:
   ```
   return options.has(includeOpt)
                   ? Optional.of(options.valueOf(includeOpt))
                   : Optional.ofNullable(options.valueOf(whitelistOpt));
   ```



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

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to