You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/06/09 13:33:19 UTC

[GitHub] [kafka] Moovlin opened a new pull request #10853: KAFKA-12811: kafka-topics.sh should let the user know they cannot adj…ust the replication factor for a topic using the --alter flag and not warn about missing the --partition flag

Moovlin opened a new pull request #10853:
URL: https://github.com/apache/kafka/pull/10853


   KAFKA-12811: kafka-topics.sh should let the user know they cannot adj…ust the replication factor for a topic using the --alter flag and not warn about missing the --partition flag
   
   This work is my own and I license it to the project. 
   
   *More detailed description of your change,
   if necessary. The PR title and PR message become
   the squashed commit message, so use a separate
   comment to ping reviewers.*
   
   
   *Summary of testing strategy (including rationale)
   for the feature or bug fix. Unit and/or integration
   tests are expected for any behaviour change and
   system tests should be considered for larger changes.*
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
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] [kafka] Moovlin commented on a change in pull request #10853: KAFKA-12811: kafka-topics.sh should let the user know they cannot adj…ust the replication factor for a topic using the --alter flag and not warn about missing the --partition flag

Posted by GitBox <gi...@apache.org>.
Moovlin commented on a change in pull request #10853:
URL: https://github.com/apache/kafka/pull/10853#discussion_r648550375



##########
File path: core/src/main/scala/kafka/admin/TopicCommand.scala
##########
@@ -574,6 +574,7 @@ object TopicCommand extends Logging {
       if (has(alterOpt)) {
         CommandLineUtils.checkInvalidArgsSet(parser, options, Set(bootstrapServerOpt, configOpt), Set(alterOpt),
         Some(kafkaConfigsCanAlterTopicConfigsViaBootstrapServer))
+        CommandLineUtils.checkInvalidArgs(parser, options, replicationFactorOpt, Set(alterOpt))

Review comment:
       I realize that I pushed a change in my branch already which doesn't fix this as I expected (my testing procedure was incorrect). Thinking about this change more though. Should it be that replication-factor cannot be used with replica-assignment? As I understand it, replication-assignment is the accepted way to adjust the replication factor for a particular topic and this just exposes a more convenient way to make that change. Please correct my understanding if it's 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] [kafka] wenbingshen commented on a change in pull request #10853: KAFKA-12811: kafka-topics.sh should let the user know they cannot adj…ust the replication factor for a topic using the --alter flag and not warn about missing the --partition flag

Posted by GitBox <gi...@apache.org>.
wenbingshen commented on a change in pull request #10853:
URL: https://github.com/apache/kafka/pull/10853#discussion_r648413266



##########
File path: core/src/main/scala/kafka/admin/TopicCommand.scala
##########
@@ -574,6 +574,7 @@ object TopicCommand extends Logging {
       if (has(alterOpt)) {
         CommandLineUtils.checkInvalidArgsSet(parser, options, Set(bootstrapServerOpt, configOpt), Set(alterOpt),
         Some(kafkaConfigsCanAlterTopicConfigsViaBootstrapServer))
+        CommandLineUtils.checkInvalidArgs(parser, options, replicationFactorOpt, Set(alterOpt))

Review comment:
       It’s okay if you just indicate that --alter cannot be used to update the replication-factor. Let's see what others say. :)




-- 
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] [kafka] wenbingshen commented on a change in pull request #10853: KAFKA-12811: kafka-topics.sh should let the user know they cannot adj…ust the replication factor for a topic using the --alter flag and not warn about missing the --partition flag

Posted by GitBox <gi...@apache.org>.
wenbingshen commented on a change in pull request #10853:
URL: https://github.com/apache/kafka/pull/10853#discussion_r648397088



##########
File path: core/src/main/scala/kafka/admin/TopicCommand.scala
##########
@@ -574,6 +574,7 @@ object TopicCommand extends Logging {
       if (has(alterOpt)) {
         CommandLineUtils.checkInvalidArgsSet(parser, options, Set(bootstrapServerOpt, configOpt), Set(alterOpt),
         Some(kafkaConfigsCanAlterTopicConfigsViaBootstrapServer))
+        CommandLineUtils.checkInvalidArgs(parser, options, replicationFactorOpt, Set(alterOpt))

Review comment:
       The following command still prompts `Missing required argument "[partitions]"`: `./bin/kafka-topics.sh --bootstrap-server `hostname`:9092 --alter --topic test-01 --replica-assignment 1002:1001,1003:1002,1001:1003,1001:1002,1002:1003,1003:1001,1002:1003 --replication-factor 3`




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