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/07/17 22:51:56 UTC

[GitHub] [kafka] mjsax opened a new pull request #11075: MINOR: Move off deprecated APIs in StreamsResetter

mjsax opened a new pull request #11075:
URL: https://github.com/apache/kafka/pull/11075


   Call for review @bbejeck 


-- 
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] mjsax commented on a change in pull request #11075: MINOR: Move off deprecated APIs in StreamsResetter

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



##########
File path: core/src/main/scala/kafka/tools/StreamsResetter.java
##########
@@ -655,7 +655,7 @@ public void doDelete(final List<String> topicsToDelete,
                          final Admin adminClient) {
         boolean hasDeleteErrors = false;
         final DeleteTopicsResult deleteTopicsResult = adminClient.deleteTopics(topicsToDelete);
-        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.values();
+        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.topicNameValues();

Review comment:
       Deprecated via https://github.com/apache/kafka/commit/cee2e975d124da0a2bdc0065a3172ce31e036fa0
   
   Could not find the corresponding KIP... \cc @jolshan @hachikuji




-- 
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] jolshan commented on a change in pull request #11075: MINOR: Move off deprecated APIs in StreamsResetter

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



##########
File path: core/src/main/scala/kafka/tools/StreamsResetter.java
##########
@@ -655,7 +655,7 @@ public void doDelete(final List<String> topicsToDelete,
                          final Admin adminClient) {
         boolean hasDeleteErrors = false;
         final DeleteTopicsResult deleteTopicsResult = adminClient.deleteTopics(topicsToDelete);
-        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.values();
+        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.topicNameValues();

Review comment:
       This is part of KIP-516

##########
File path: core/src/main/scala/kafka/tools/StreamsResetter.java
##########
@@ -655,7 +655,7 @@ public void doDelete(final List<String> topicsToDelete,
                          final Admin adminClient) {
         boolean hasDeleteErrors = false;
         final DeleteTopicsResult deleteTopicsResult = adminClient.deleteTopics(topicsToDelete);
-        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.values();
+        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.topicNameValues();

Review comment:
       I believe the jira was under the kip's main jira, but I can make the link clearer.




-- 
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] showuon commented on a change in pull request #11075: MINOR: Move off deprecated APIs in StreamsResetter

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



##########
File path: core/src/main/scala/kafka/tools/StreamsResetter.java
##########
@@ -304,7 +304,7 @@ private void parseArguments(final String[] args) {
             optionParser,
             options,
             option,
-            JavaConverters.asScalaSetConverter(invalidOptions).asScala());
+            CollectionConverters.SetHasAsScala(invalidOptions).asScala());

Review comment:
       This is deprecated by scala (scala 2.12, not 0.12 :)). Here's the note from scala doc in 2.12.0, to explain why it is deprecated:
   >  The transparent conversions provided here are considered fragile because they can result in unexpected behavior and performance.
   
   > Therefore, this API has been deprecated and JavaConverters should be used instead.
   
   I think that time, `CollectionConverters` haven't existed, so suggest to use `JavaConverters` instead. 
   REF: https://www.scala-lang.org/api/2.12.0/scala/collection/JavaConversions$.html




-- 
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] jolshan commented on a change in pull request #11075: MINOR: Move off deprecated APIs in StreamsResetter

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



##########
File path: core/src/main/scala/kafka/tools/StreamsResetter.java
##########
@@ -655,7 +655,7 @@ public void doDelete(final List<String> topicsToDelete,
                          final Admin adminClient) {
         boolean hasDeleteErrors = false;
         final DeleteTopicsResult deleteTopicsResult = adminClient.deleteTopics(topicsToDelete);
-        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.values();
+        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.topicNameValues();

Review comment:
       I believe the jira was under the kip's main jira, but I can make the link clearer.




-- 
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] mjsax commented on a change in pull request #11075: MINOR: Move off deprecated APIs in StreamsResetter

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



##########
File path: core/src/main/scala/kafka/tools/StreamsResetter.java
##########
@@ -655,7 +655,7 @@ public void doDelete(final List<String> topicsToDelete,
                          final Admin adminClient) {
         boolean hasDeleteErrors = false;
         final DeleteTopicsResult deleteTopicsResult = adminClient.deleteTopics(topicsToDelete);
-        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.values();
+        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.topicNameValues();

Review comment:
       Thanks! Can we cross link KIP wiki page and Jiras to make it easier to find?

##########
File path: core/src/main/scala/kafka/tools/StreamsResetter.java
##########
@@ -655,7 +655,7 @@ public void doDelete(final List<String> topicsToDelete,
                          final Admin adminClient) {
         boolean hasDeleteErrors = false;
         final DeleteTopicsResult deleteTopicsResult = adminClient.deleteTopics(topicsToDelete);
-        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.values();
+        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.topicNameValues();

Review comment:
       Thanks!
   
   It's also best, to mention a KIP in the commit message. I guess @hachikuji just forgot to add it?




-- 
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] jolshan commented on a change in pull request #11075: MINOR: Move off deprecated APIs in StreamsResetter

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



##########
File path: core/src/main/scala/kafka/tools/StreamsResetter.java
##########
@@ -655,7 +655,7 @@ public void doDelete(final List<String> topicsToDelete,
                          final Admin adminClient) {
         boolean hasDeleteErrors = false;
         final DeleteTopicsResult deleteTopicsResult = adminClient.deleteTopics(topicsToDelete);
-        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.values();
+        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.topicNameValues();

Review comment:
       This is part of KIP-516




-- 
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] showuon commented on a change in pull request #11075: MINOR: Move off deprecated APIs in StreamsResetter

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



##########
File path: core/src/main/scala/kafka/tools/StreamsResetter.java
##########
@@ -304,7 +304,7 @@ private void parseArguments(final String[] args) {
             optionParser,
             options,
             option,
-            JavaConverters.asScalaSetConverter(invalidOptions).asScala());
+            CollectionConverters.SetHasAsScala(invalidOptions).asScala());

Review comment:
       Sorry, I was wrong. Have a 2nd look, and found `CollectionConverters` doesn't exist in scala 2.12. So, this change will not be compatible to scala 2.12. This change should be reverted. 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] mjsax commented on a change in pull request #11075: MINOR: Move off deprecated APIs in StreamsResetter

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



##########
File path: core/src/main/scala/kafka/tools/StreamsResetter.java
##########
@@ -655,7 +655,7 @@ public void doDelete(final List<String> topicsToDelete,
                          final Admin adminClient) {
         boolean hasDeleteErrors = false;
         final DeleteTopicsResult deleteTopicsResult = adminClient.deleteTopics(topicsToDelete);
-        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.values();
+        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.topicNameValues();

Review comment:
       Thanks! Can we cross link KIP wiki page and Jiras to make it easier to find?




-- 
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] jolshan commented on a change in pull request #11075: MINOR: Move off deprecated APIs in StreamsResetter

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



##########
File path: core/src/main/scala/kafka/tools/StreamsResetter.java
##########
@@ -655,7 +655,7 @@ public void doDelete(final List<String> topicsToDelete,
                          final Admin adminClient) {
         boolean hasDeleteErrors = false;
         final DeleteTopicsResult deleteTopicsResult = adminClient.deleteTopics(topicsToDelete);
-        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.values();
+        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.topicNameValues();

Review comment:
       This is part of KIP-516

##########
File path: core/src/main/scala/kafka/tools/StreamsResetter.java
##########
@@ -655,7 +655,7 @@ public void doDelete(final List<String> topicsToDelete,
                          final Admin adminClient) {
         boolean hasDeleteErrors = false;
         final DeleteTopicsResult deleteTopicsResult = adminClient.deleteTopics(topicsToDelete);
-        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.values();
+        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.topicNameValues();

Review comment:
       I believe the jira was under the kip's main jira, but I can make the link clearer.




-- 
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] mjsax merged pull request #11075: MINOR: Move off deprecated APIs in StreamsResetter

Posted by GitBox <gi...@apache.org>.
mjsax merged pull request #11075:
URL: https://github.com/apache/kafka/pull/11075


   


-- 
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] mjsax commented on a change in pull request #11075: MINOR: Move off deprecated APIs in StreamsResetter

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



##########
File path: core/src/main/scala/kafka/tools/StreamsResetter.java
##########
@@ -655,7 +655,7 @@ public void doDelete(final List<String> topicsToDelete,
                          final Admin adminClient) {
         boolean hasDeleteErrors = false;
         final DeleteTopicsResult deleteTopicsResult = adminClient.deleteTopics(topicsToDelete);
-        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.values();
+        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.topicNameValues();

Review comment:
       Thanks!
   
   It's also best, to mention a KIP in the commit message. I guess @hachikuji just forgot to add it?




-- 
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] showuon commented on a change in pull request #11075: MINOR: Move off deprecated APIs in StreamsResetter

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



##########
File path: core/src/main/scala/kafka/tools/StreamsResetter.java
##########
@@ -304,7 +304,7 @@ private void parseArguments(final String[] args) {
             optionParser,
             options,
             option,
-            JavaConverters.asScalaSetConverter(invalidOptions).asScala());
+            CollectionConverters.SetHasAsScala(invalidOptions).asScala());

Review comment:
       Sorry, I was wrong. Have a 2nd look, and found `CollectionConverters` doesn't exist in scala 2.12. So, this change will not be compatible to scala 2.12. This change should be reverted, or maybe use `JavaConverters`? 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] mjsax commented on a change in pull request #11075: MINOR: Move off deprecated APIs in StreamsResetter

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



##########
File path: core/src/main/scala/kafka/tools/StreamsResetter.java
##########
@@ -655,7 +655,7 @@ public void doDelete(final List<String> topicsToDelete,
                          final Admin adminClient) {
         boolean hasDeleteErrors = false;
         final DeleteTopicsResult deleteTopicsResult = adminClient.deleteTopics(topicsToDelete);
-        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.values();
+        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.topicNameValues();

Review comment:
       Thanks! Can we cross link KIP wiki page and Jiras to make it easier to find?

##########
File path: core/src/main/scala/kafka/tools/StreamsResetter.java
##########
@@ -655,7 +655,7 @@ public void doDelete(final List<String> topicsToDelete,
                          final Admin adminClient) {
         boolean hasDeleteErrors = false;
         final DeleteTopicsResult deleteTopicsResult = adminClient.deleteTopics(topicsToDelete);
-        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.values();
+        final Map<String, KafkaFuture<Void>> results = deleteTopicsResult.topicNameValues();

Review comment:
       Thanks!
   
   It's also best, to mention a KIP in the commit message. I guess @hachikuji just forgot to add it?




-- 
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] mjsax commented on a change in pull request #11075: MINOR: Move off deprecated APIs in StreamsResetter

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



##########
File path: core/src/main/scala/kafka/tools/StreamsResetter.java
##########
@@ -304,7 +304,7 @@ private void parseArguments(final String[] args) {
             optionParser,
             options,
             option,
-            JavaConverters.asScalaSetConverter(invalidOptions).asScala());
+            CollectionConverters.SetHasAsScala(invalidOptions).asScala());

Review comment:
       Seems to be a result of KIP-751 (drop support for Scala 0.12) ?




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