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/29 09:52:46 UTC

[GitHub] [kafka] OmniaGM opened a new pull request #10937: [WIP] KAFKA-10587 Rename kafka-mirror-maker CLI command line arguments for KIP-629

OmniaGM opened a new pull request #10937:
URL: https://github.com/apache/kafka/pull/10937


   [KAFKA-10587](https://issues.apache.org/jira/browse/KAFKA-10587) Rename kafka-mirror-maker CLI command line arguments for KIP-629
   Remove whitelist term from kafka-mirror-maker cli command by renaming whitelist to include
   
   ### 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.

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

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

[GitHub] [kafka] OmniaGM commented on a change in pull request #10937: KAFKA-10587 Rename kafka-mirror-maker CLI command line arguments for KIP-629

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



##########
File path: core/src/main/scala/kafka/tools/MirrorMaker.scala
##########
@@ -454,8 +454,8 @@ object MirrorMaker extends Logging with KafkaMetricsGroup {
       .ofType(classOf[java.lang.Integer])
       .defaultsTo(1)
 
-    val whitelistOpt = parser.accepts("whitelist",
-      "Whitelist of topics to mirror.")
+    val includeOpt = parser.accepts("include",

Review comment:
       I added it back and handled both. Now the command will have 
   ```
   --whitelist <String: Java regex          DEPRECATED, use --include instead;    
     (String)>                                ignored if --include specified. List
                                              of included topics to mirror. 
   --include <String: Java regex (String)>  List of included topics to mirror.    
   ```
   




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

[GitHub] [kafka] OmniaGM commented on pull request #10937: KAFKA-10587 Rename kafka-mirror-maker CLI command line arguments for KIP-629

Posted by GitBox <gi...@apache.org>.
OmniaGM commented on pull request #10937:
URL: https://github.com/apache/kafka/pull/10937#issuecomment-872226653


   > MirrorMakerIntegrationTest
   
   @showuon Thanks for having a look, the pr already includes `MirrorMakerIntegrationTest.scala`. I did a quick search and from the look of it, this is the only MM2 test file that mentioned racial terms. 


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

[GitHub] [kafka] xvrl commented on a change in pull request #10937: KAFKA-10587 Rename kafka-mirror-maker CLI command line arguments for KIP-629

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



##########
File path: core/src/main/scala/kafka/tools/MirrorMaker.scala
##########
@@ -455,7 +455,13 @@ object MirrorMaker extends Logging with KafkaMetricsGroup {
       .defaultsTo(1)
 
     val whitelistOpt = parser.accepts("whitelist",
-      "Whitelist of topics to mirror.")
+      "DEPRECATED, use --include instead; ignored if --include specified. List of included topics to mirror.")

Review comment:
       The term "included" seems redundant ```suggestion
         "DEPRECATED, use --include instead; ignored if --include specified. List of topics to mirror.")
   ```

##########
File path: core/src/main/scala/kafka/tools/MirrorMaker.scala
##########
@@ -455,7 +455,13 @@ object MirrorMaker extends Logging with KafkaMetricsGroup {
       .defaultsTo(1)
 
     val whitelistOpt = parser.accepts("whitelist",
-      "Whitelist of topics to mirror.")
+      "DEPRECATED, use --include instead; ignored if --include specified. List of included topics to mirror.")

Review comment:
       The term "included" seems redundant
   ```suggestion
         "DEPRECATED, use --include instead; ignored if --include specified. List of topics to mirror.")
   ```




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

[GitHub] [kafka] OmniaGM edited a comment on pull request #10937: KAFKA-10587 Rename kafka-mirror-maker CLI command line arguments for KIP-629

Posted by GitBox <gi...@apache.org>.
OmniaGM edited a comment on pull request #10937:
URL: https://github.com/apache/kafka/pull/10937#issuecomment-872226653


   > Overall LGTM. Could you please also update the test file for MirrorMaker? (i.e. MirrorMakerIntegrationTest.scala) Thanks.
   
   @showuon Thanks for having a look, the pr already includes `MirrorMakerIntegrationTest.scala`. I did a quick search and from the look of it, this is the only MM2 test file that mentioned racial terms. 


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

[GitHub] [kafka] xvrl commented on a change in pull request #10937: KAFKA-10587 Rename kafka-mirror-maker CLI command line arguments for KIP-629

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



##########
File path: core/src/main/scala/kafka/tools/MirrorMaker.scala
##########
@@ -455,7 +455,13 @@ object MirrorMaker extends Logging with KafkaMetricsGroup {
       .defaultsTo(1)
 
     val whitelistOpt = parser.accepts("whitelist",
-      "Whitelist of topics to mirror.")
+      "DEPRECATED, use --include instead; ignored if --include specified. List of included topics to mirror.")
+      .withRequiredArg()
+      .describedAs("Java regex (String)")
+      .ofType(classOf[String])
+
+    val includeOpt = parser.accepts("include",
+      "List of included topics to mirror.")

Review comment:
       same as above
   ```suggestion
         "List of topics to mirror.")
   ```




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

[GitHub] [kafka] OmniaGM commented on pull request #10937: KAFKA-10587 Rename kafka-mirror-maker CLI command line arguments for KIP-629

Posted by GitBox <gi...@apache.org>.
OmniaGM commented on pull request #10937:
URL: https://github.com/apache/kafka/pull/10937#issuecomment-872098303


   Hi, @xvrl, @mimaison can you please have a look into this? 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

[GitHub] [kafka] gwenshap closed pull request #10937: KAFKA-10587 Rename kafka-mirror-maker CLI command line arguments for KIP-629

Posted by GitBox <gi...@apache.org>.
gwenshap closed pull request #10937:
URL: https://github.com/apache/kafka/pull/10937


   


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

[GitHub] [kafka] xvrl commented on a change in pull request #10937: KAFKA-10587 Rename kafka-mirror-maker CLI command line arguments for KIP-629

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



##########
File path: core/src/main/scala/kafka/tools/MirrorMaker.scala
##########
@@ -455,7 +455,13 @@ object MirrorMaker extends Logging with KafkaMetricsGroup {
       .defaultsTo(1)
 
     val whitelistOpt = parser.accepts("whitelist",
-      "Whitelist of topics to mirror.")
+      "DEPRECATED, use --include instead; ignored if --include specified. List of included topics to mirror.")

Review comment:
       nit, the term "included" seems redundant
   ```suggestion
         "DEPRECATED, use --include instead; ignored if --include specified. List of topics to mirror.")
   ```




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

[GitHub] [kafka] xvrl commented on a change in pull request #10937: KAFKA-10587 Rename kafka-mirror-maker CLI command line arguments for KIP-629

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



##########
File path: core/src/main/scala/kafka/tools/MirrorMaker.scala
##########
@@ -454,8 +454,8 @@ object MirrorMaker extends Logging with KafkaMetricsGroup {
       .ofType(classOf[java.lang.Integer])
       .defaultsTo(1)
 
-    val whitelistOpt = parser.accepts("whitelist",
-      "Whitelist of topics to mirror.")
+    val includeOpt = parser.accepts("include",

Review comment:
       we should deprecate the old argument, but provide backwards compatibility until we remove it in a future release.




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