You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "Yunyung (via GitHub)" <gi...@apache.org> on 2025/08/29 16:42:02 UTC

[PR] MINOR: Update inconsistent description for bootstrap-server in docs and command-line [kafka]

Yunyung opened a new pull request, #20445:
URL: https://github.com/apache/kafka/pull/20445

   ### **kafka-streams-application-reset.sh**
   - Update docs `--bootstrap-servers` has been removed.
   - Fix `--bootstrap-server` is required but not apply to StreamsResetter.java.
   
   ### **kafka-producer-perf-test.sh**
   - Update the metavar name for `--bootstrap-server`


-- 
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-unsubscribe@kafka.apache.org

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


Re: [PR] MINOR: Update inconsistent description for kafka-streams-application-reset and kafka-producer-perf-test [kafka]

Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 merged PR #20445:
URL: https://github.com/apache/kafka/pull/20445


-- 
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-unsubscribe@kafka.apache.org

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


Re: [PR] MINOR: Update inconsistent description for kafka-streams-application-reset and kafka-producer-perf-test [kafka]

Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #20445:
URL: https://github.com/apache/kafka/pull/20445#discussion_r2388354347


##########
tools/src/main/java/org/apache/kafka/tools/StreamsResetter.java:
##########
@@ -585,15 +582,16 @@ private static class StreamsResetterOptions extends CommandDefaultOptions {
 
         public StreamsResetterOptions(String[] args) {
             super(args);
-            applicationIdOption = parser.accepts("application-id", "The Kafka Streams application ID (application.id).")
+            applicationIdOption = parser.accepts("application-id", "REQUIRED: The Kafka Streams application ID (application.id).")
                 .withRequiredArg()
                 .ofType(String.class)
                 .describedAs("id")
                 .required();
-            bootstrapServerOption = parser.accepts("bootstrap-server", "The server(s) to connect to. The broker list string in the form HOST1:PORT1,HOST2:PORT2. (default: localhost:9092)")
+            bootstrapServerOption = parser.accepts("bootstrap-server", "REQUIRED: The server(s) to connect to. The broker list string in the form HOST1:PORT1,HOST2:PORT2.")
                 .withRequiredArg()
                 .ofType(String.class)
-                .describedAs("server to connect to");
+                .describedAs("server to connect to")
+                .required();

Review Comment:
   We should keep the word `REQUIRED` since the `bootstrapServerOption` is now completely required. That should be addressed in #16437 :smile: 



-- 
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-unsubscribe@kafka.apache.org

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


Re: [PR] MINOR: Update inconsistent description for kafka-streams-application-reset and kafka-producer-perf-test [kafka]

Posted by "Yunyung (via GitHub)" <gi...@apache.org>.
Yunyung commented on code in PR #20445:
URL: https://github.com/apache/kafka/pull/20445#discussion_r2388506027


##########
tools/src/main/java/org/apache/kafka/tools/StreamsResetter.java:
##########
@@ -585,15 +582,16 @@ private static class StreamsResetterOptions extends CommandDefaultOptions {
 
         public StreamsResetterOptions(String[] args) {
             super(args);
-            applicationIdOption = parser.accepts("application-id", "The Kafka Streams application ID (application.id).")
+            applicationIdOption = parser.accepts("application-id", "REQUIRED: The Kafka Streams application ID (application.id).")
                 .withRequiredArg()
                 .ofType(String.class)
                 .describedAs("id")
                 .required();
-            bootstrapServerOption = parser.accepts("bootstrap-server", "The server(s) to connect to. The broker list string in the form HOST1:PORT1,HOST2:PORT2. (default: localhost:9092)")
+            bootstrapServerOption = parser.accepts("bootstrap-server", "REQUIRED: The server(s) to connect to. The broker list string in the form HOST1:PORT1,HOST2:PORT2.")
                 .withRequiredArg()
                 .ofType(String.class)
-                .describedAs("server to connect to");
+                .describedAs("server to connect to")
+                .required();

Review Comment:
   Sure. I've reverted the latest commit, 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.

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

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


Re: [PR] MINOR: Update inconsistent description for bootstrap-server in docs and command-line [kafka]

Posted by "Yunyung (via GitHub)" <gi...@apache.org>.
Yunyung commented on PR #20445:
URL: https://github.com/apache/kafka/pull/20445#issuecomment-3239201951

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

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

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


Re: [PR] MINOR: Update inconsistent description for bootstrap-server in docs and command-line [kafka]

Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on code in PR #20445:
URL: https://github.com/apache/kafka/pull/20445#discussion_r2312203818


##########
tools/src/main/java/org/apache/kafka/tools/StreamsResetter.java:
##########
@@ -653,6 +650,7 @@ public StreamsResetterOptions(String[] args) {
                 if (CommandLineUtils.isPrintVersionNeeded(this)) {
                     CommandLineUtils.printVersionAndExit();
                 }
+                CommandLineUtils.checkRequiredArgs(parser, options, bootstrapServerOption);

Review Comment:
   Could you also add a test to verify that the `bootstrap-server` argument is required and that the command fails with a proper error message if it is missing?



-- 
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-unsubscribe@kafka.apache.org

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


Re: [PR] MINOR: Update inconsistent description for kafka-streams-application-reset and kafka-producer-perf-test [kafka]

Posted by "Yunyung (via GitHub)" <gi...@apache.org>.
Yunyung commented on code in PR #20445:
URL: https://github.com/apache/kafka/pull/20445#discussion_r2421689430


##########
tools/src/main/java/org/apache/kafka/tools/StreamsResetter.java:
##########
@@ -585,15 +582,16 @@ private static class StreamsResetterOptions extends CommandDefaultOptions {
 
         public StreamsResetterOptions(String[] args) {
             super(args);
-            applicationIdOption = parser.accepts("application-id", "The Kafka Streams application ID (application.id).")
+            applicationIdOption = parser.accepts("application-id", "REQUIRED: The Kafka Streams application ID (application.id).")
                 .withRequiredArg()
                 .ofType(String.class)
                 .describedAs("id")
                 .required();
-            bootstrapServerOption = parser.accepts("bootstrap-server", "The server(s) to connect to. The broker list string in the form HOST1:PORT1,HOST2:PORT2. (default: localhost:9092)")
+            bootstrapServerOption = parser.accepts("bootstrap-server", "REQUIRED: The server(s) to connect to. The broker list string in the form HOST1:PORT1,HOST2:PORT2.")
                 .withRequiredArg()
                 .ofType(String.class)
-                .describedAs("server to connect to");
+                .describedAs("server to connect to")
+                .required();

Review Comment:
   No worries, I was lost too, haha.
   
   Updated.



-- 
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-unsubscribe@kafka.apache.org

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


Re: [PR] MINOR: Update inconsistent description for bootstrap-server in docs and command-line [kafka]

Posted by "Yunyung (via GitHub)" <gi...@apache.org>.
Yunyung commented on code in PR #20445:
URL: https://github.com/apache/kafka/pull/20445#discussion_r2386474990


##########
tools/src/main/java/org/apache/kafka/tools/StreamsResetter.java:
##########
@@ -585,15 +582,16 @@ private static class StreamsResetterOptions extends CommandDefaultOptions {
 
         public StreamsResetterOptions(String[] args) {
             super(args);
-            applicationIdOption = parser.accepts("application-id", "The Kafka Streams application ID (application.id).")
+            applicationIdOption = parser.accepts("application-id", "REQUIRED: The Kafka Streams application ID (application.id).")
                 .withRequiredArg()
                 .ofType(String.class)
                 .describedAs("id")
                 .required();
-            bootstrapServerOption = parser.accepts("bootstrap-server", "The server(s) to connect to. The broker list string in the form HOST1:PORT1,HOST2:PORT2. (default: localhost:9092)")
+            bootstrapServerOption = parser.accepts("bootstrap-server", "REQUIRED: The server(s) to connect to. The broker list string in the form HOST1:PORT1,HOST2:PORT2.")
                 .withRequiredArg()
                 .ofType(String.class)
-                .describedAs("server to connect to");
+                .describedAs("server to connect to")
+                .required();

Review Comment:
   Updated. And also update the screenshot above.



-- 
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-unsubscribe@kafka.apache.org

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


Re: [PR] MINOR: Update inconsistent description for bootstrap-server in docs and command-line [kafka]

Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #20445:
URL: https://github.com/apache/kafka/pull/20445#discussion_r2376428497


##########
tools/src/main/java/org/apache/kafka/tools/StreamsResetter.java:
##########
@@ -585,15 +582,16 @@ private static class StreamsResetterOptions extends CommandDefaultOptions {
 
         public StreamsResetterOptions(String[] args) {
             super(args);
-            applicationIdOption = parser.accepts("application-id", "The Kafka Streams application ID (application.id).")
+            applicationIdOption = parser.accepts("application-id", "REQUIRED: The Kafka Streams application ID (application.id).")
                 .withRequiredArg()
                 .ofType(String.class)
                 .describedAs("id")
                 .required();
-            bootstrapServerOption = parser.accepts("bootstrap-server", "The server(s) to connect to. The broker list string in the form HOST1:PORT1,HOST2:PORT2. (default: localhost:9092)")
+            bootstrapServerOption = parser.accepts("bootstrap-server", "REQUIRED: The server(s) to connect to. The broker list string in the form HOST1:PORT1,HOST2:PORT2.")
                 .withRequiredArg()
                 .ofType(String.class)
-                .describedAs("server to connect to");
+                .describedAs("server to connect to")
+                .required();

Review Comment:
   Is this breaking change included by the KIP? 



-- 
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-unsubscribe@kafka.apache.org

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


Re: [PR] MINOR: Update inconsistent description for bootstrap-server in docs and command-line [kafka]

Posted by "Yunyung (via GitHub)" <gi...@apache.org>.
Yunyung commented on PR #20445:
URL: https://github.com/apache/kafka/pull/20445#issuecomment-3237634083

   ### For kafka-streams-application-reset.sh
   
   Before:
   <img width="2720" height="1970" alt="Screenshot from 2025-08-30 00-41-46" src="https://github.com/user-attachments/assets/616dbd94-0364-4aae-9217-4f6da5bf036b" />
   
   
   After:
   <img width="2616" height="1820" alt="Screenshot from 2025-08-30 00-27-51" src="https://github.com/user-attachments/assets/3720914d-a539-4c3a-8e08-1b12ca83434e" />
   


-- 
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-unsubscribe@kafka.apache.org

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


Re: [PR] MINOR: Update inconsistent description for kafka-streams-application-reset and kafka-producer-perf-test [kafka]

Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #20445:
URL: https://github.com/apache/kafka/pull/20445#issuecomment-3476788242

   @frankvicky @DL1231 @m1a2st Sorry about that - I forgot to add you to reviewers. my bad :(


-- 
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-unsubscribe@kafka.apache.org

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


Re: [PR] MINOR: Update inconsistent description for kafka-streams-application-reset and kafka-producer-perf-test [kafka]

Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #20445:
URL: https://github.com/apache/kafka/pull/20445#discussion_r2414653658


##########
tools/src/main/java/org/apache/kafka/tools/StreamsResetter.java:
##########
@@ -585,15 +582,16 @@ private static class StreamsResetterOptions extends CommandDefaultOptions {
 
         public StreamsResetterOptions(String[] args) {
             super(args);
-            applicationIdOption = parser.accepts("application-id", "The Kafka Streams application ID (application.id).")
+            applicationIdOption = parser.accepts("application-id", "REQUIRED: The Kafka Streams application ID (application.id).")
                 .withRequiredArg()
                 .ofType(String.class)
                 .describedAs("id")
                 .required();
-            bootstrapServerOption = parser.accepts("bootstrap-server", "The server(s) to connect to. The broker list string in the form HOST1:PORT1,HOST2:PORT2. (default: localhost:9092)")
+            bootstrapServerOption = parser.accepts("bootstrap-server", "REQUIRED: The server(s) to connect to. The broker list string in the form HOST1:PORT1,HOST2:PORT2.")
                 .withRequiredArg()
                 .ofType(String.class)
-                .describedAs("server to connect to");
+                .describedAs("server to connect to")
+                .required();

Review Comment:
   Sorry, my last comment was incorrect. It has a default value, so the config is definitely optional
   ```
   String bootstrapServerValue = "localhost:9092";
   ```



-- 
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-unsubscribe@kafka.apache.org

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


Re: [PR] MINOR: Update inconsistent description for bootstrap-server in docs and command-line [kafka]

Posted by "DL1231 (via GitHub)" <gi...@apache.org>.
DL1231 commented on code in PR #20445:
URL: https://github.com/apache/kafka/pull/20445#discussion_r2311764627


##########
tools/src/main/java/org/apache/kafka/tools/ProducerPerformance.java:
##########
@@ -241,7 +241,7 @@ static ArgumentParser argParser() {
                 .action(store())
                 .required(false)
                 .type(String.class)
-                .metavar("BOOTSTRAP-SERVERS")
+                .metavar("BOOTSTRAP-SERVER")
                 .dest("bootstrapServers")

Review Comment:
   nit: bootstrapServer
   
   ConfigPostProcessor.bootstrapServers -> bootstrapServer



-- 
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-unsubscribe@kafka.apache.org

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


Re: [PR] MINOR: Update inconsistent description for bootstrap-server in docs and command-line [kafka]

Posted by "Yunyung (via GitHub)" <gi...@apache.org>.
Yunyung commented on code in PR #20445:
URL: https://github.com/apache/kafka/pull/20445#discussion_r2377340161


##########
tools/src/main/java/org/apache/kafka/tools/StreamsResetter.java:
##########
@@ -585,15 +582,16 @@ private static class StreamsResetterOptions extends CommandDefaultOptions {
 
         public StreamsResetterOptions(String[] args) {
             super(args);
-            applicationIdOption = parser.accepts("application-id", "The Kafka Streams application ID (application.id).")
+            applicationIdOption = parser.accepts("application-id", "REQUIRED: The Kafka Streams application ID (application.id).")
                 .withRequiredArg()
                 .ofType(String.class)
                 .describedAs("id")
                 .required();
-            bootstrapServerOption = parser.accepts("bootstrap-server", "The server(s) to connect to. The broker list string in the form HOST1:PORT1,HOST2:PORT2. (default: localhost:9092)")
+            bootstrapServerOption = parser.accepts("bootstrap-server", "REQUIRED: The server(s) to connect to. The broker list string in the form HOST1:PORT1,HOST2:PORT2.")
                 .withRequiredArg()
                 .ofType(String.class)
-                .describedAs("server to connect to");
+                .describedAs("server to connect to")
+                .required();

Review Comment:
   I'll revert the change that requires bootstrapServer, since the previous [KIP-865](https://cwiki.apache.org/confluence/display/KAFKA/KIP-865%3A+Support+--bootstrap-server+in+kafka-streams-application-reset) is ambiguous and highly dependent on the implementation. It's better not to introduce the breaking change here. 
   
   That is, this PR only updates the docs and text. 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.

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

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


Re: [PR] MINOR: Update inconsistent description for bootstrap-server in docs and command-line [kafka]

Posted by "Yunyung (via GitHub)" <gi...@apache.org>.
Yunyung commented on code in PR #20445:
URL: https://github.com/apache/kafka/pull/20445#discussion_r2311840491


##########
tools/src/main/java/org/apache/kafka/tools/ProducerPerformance.java:
##########
@@ -241,7 +241,7 @@ static ArgumentParser argParser() {
                 .action(store())
                 .required(false)
                 .type(String.class)
-                .metavar("BOOTSTRAP-SERVERS")
+                .metavar("BOOTSTRAP-SERVER")
                 .dest("bootstrapServers")

Review Comment:
   Done. Thanks for the review.



-- 
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-unsubscribe@kafka.apache.org

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


Re: [PR] MINOR: Update inconsistent description for bootstrap-server in docs and command-line [kafka]

Posted by "Yunyung (via GitHub)" <gi...@apache.org>.
Yunyung commented on PR #20445:
URL: https://github.com/apache/kafka/pull/20445#issuecomment-3240233434

   I’ve added the test and made slight updates to the code and docs. Also updated the "After" screenshot in #2. Thanks for the review.


-- 
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-unsubscribe@kafka.apache.org

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