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 2020/05/15 11:56:08 UTC

[GitHub] [kafka] akatona84 opened a new pull request #8673: KAFKA-9992: EmbeddedKafkaCluster.deleteTopicAndWait not working with kafka_2.13

akatona84 opened a new pull request #8673:
URL: https://github.com/apache/kafka/pull/8673


   Eliminating JavaConverters in EmbeddedKafkaCluster
   
   *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] akatona84 commented on pull request #8673: KAFKA-9992: EmbeddedKafkaCluster.deleteTopicAndWait not working with kafka_2.13

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


   retest this please


----------------------------------------------------------------
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] vvcephei merged pull request #8673: KAFKA-9992: EmbeddedKafkaCluster.deleteTopicAndWait not working with kafka_2.13

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


   


----------------------------------------------------------------
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] vvcephei commented on pull request #8673: KAFKA-9992: EmbeddedKafkaCluster.deleteTopicAndWait not working with kafka_2.13

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


   (That last comment was to trigger the tests)


----------------------------------------------------------------
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] akatona84 commented on pull request #8673: KAFKA-9992: EmbeddedKafkaCluster.deleteTopicAndWait not working with kafka_2.13

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


   @vvcephei without the scala to java converter it would be compatible.


----------------------------------------------------------------
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] vvcephei commented on pull request #8673: KAFKA-9992: EmbeddedKafkaCluster.deleteTopicAndWait not working with kafka_2.13

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


   The tests that failed are known to be flaky and seem unrelated to this change:
       org.apache.kafka.streams.integration.EosBetaUpgradeIntegrationTest.shouldUpgradeFromEosAlphaToEosBeta[true]
       org.apache.kafka.streams.integration.EosBetaUpgradeIntegrationTest.shouldUpgradeFromEosAlphaToEosBeta[true]
       org.apache.kafka.streams.integration.EosIntegrationTest.shouldNotViolateEosIfOneTaskFailsWithState[exactly_once]


----------------------------------------------------------------
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] vvcephei commented on pull request #8673: KAFKA-9992: EmbeddedKafkaCluster.deleteTopicAndWait not working with kafka_2.13

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


   test this please


----------------------------------------------------------------
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] vvcephei commented on pull request #8673: KAFKA-9992: EmbeddedKafkaCluster.deleteTopicAndWait not working with kafka_2.13

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


   Test this please


----------------------------------------------------------------
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] akatona84 commented on a change in pull request #8673: KAFKA-9992: EmbeddedKafkaCluster.deleteTopicAndWait not working with kafka_2.13

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



##########
File path: streams/src/test/java/org/apache/kafka/streams/integration/utils/EmbeddedKafkaCluster.java
##########
@@ -297,6 +295,15 @@ public void deleteAllTopicsAndWait(final long timeoutMs) throws InterruptedExcep
         }
     }
 
+    public Set<String> getAllTopics() {
+        scala.collection.Iterator<String> topicsIterator = brokers[0].kafkaServer().zkClient().getAllTopicsInCluster().iterator();

Review comment:
       thx, i was cherry picking my change from 2.4.1 based kafka branch and config resolution did not go well, I uploaded a new, clean change.




----------------------------------------------------------------
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] vvcephei commented on a change in pull request #8673: KAFKA-9992: EmbeddedKafkaCluster.deleteTopicAndWait not working with kafka_2.13

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



##########
File path: streams/src/test/java/org/apache/kafka/streams/integration/utils/EmbeddedKafkaCluster.java
##########
@@ -314,8 +321,7 @@ private TopicsDeletedCondition(final Collection<String> topics) {
 
         @Override
         public boolean conditionMet() {
-            final Set<String> allTopics = new HashSet<>(JavaConverters.setAsJavaSetConverter(
-                brokers[0].kafkaServer().zkClient().getAllTopicsInCluster(false)).asJava());
+            final Set<String> allTopics = new HashSet<>(getAllTopics());

Review comment:
       It looks like we don’t need to make a copy here anymore




----------------------------------------------------------------
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] akatona84 commented on a change in pull request #8673: KAFKA-9992: EmbeddedKafkaCluster.deleteTopicAndWait not working with kafka_2.13

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



##########
File path: streams/src/test/java/org/apache/kafka/streams/integration/utils/EmbeddedKafkaCluster.java
##########
@@ -297,6 +295,15 @@ public void deleteAllTopicsAndWait(final long timeoutMs) throws InterruptedExcep
         }
     }
 
+    public Set<String> getAllTopics() {
+        scala.collection.Iterator<String> topicsIterator = brokers[0].kafkaServer().zkClient().getAllTopicsInCluster().iterator();

Review comment:
       thx, i was cherry picking my change from 2.4.1 based kafka branch, I uploaded a new, clean change.




----------------------------------------------------------------
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] chia7712 commented on a change in pull request #8673: KAFKA-9992: EmbeddedKafkaCluster.deleteTopicAndWait not working with kafka_2.13

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



##########
File path: streams/src/test/java/org/apache/kafka/streams/integration/utils/EmbeddedKafkaCluster.java
##########
@@ -297,6 +295,15 @@ public void deleteAllTopicsAndWait(final long timeoutMs) throws InterruptedExcep
         }
     }
 
+    public Set<String> getAllTopics() {
+        scala.collection.Iterator<String> topicsIterator = brokers[0].kafkaServer().zkClient().getAllTopicsInCluster().iterator();

Review comment:
       > getAllTopicsInCluster()
   
   ```getAllTopicsInCluster(false)```

##########
File path: streams/src/test/java/org/apache/kafka/streams/integration/utils/EmbeddedKafkaCluster.java
##########
@@ -297,6 +295,15 @@ public void deleteAllTopicsAndWait(final long timeoutMs) throws InterruptedExcep
         }
     }
 
+    public Set<String> getAllTopics() {

Review comment:
       ```getAllTopicsInCluster``` seems to be duplicate to this new method. Could we remove the former?




----------------------------------------------------------------
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] akatona84 commented on a change in pull request #8673: KAFKA-9992: EmbeddedKafkaCluster.deleteTopicAndWait not working with kafka_2.13

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



##########
File path: streams/src/test/java/org/apache/kafka/streams/integration/utils/EmbeddedKafkaCluster.java
##########
@@ -314,8 +321,7 @@ private TopicsDeletedCondition(final Collection<String> topics) {
 
         @Override
         public boolean conditionMet() {
-            final Set<String> allTopics = new HashSet<>(JavaConverters.setAsJavaSetConverter(
-                brokers[0].kafkaServer().zkClient().getAllTopicsInCluster(false)).asJava());
+            final Set<String> allTopics = new HashSet<>(getAllTopics());

Review comment:
       fixed in new commit




----------------------------------------------------------------
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] vvcephei commented on pull request #8673: KAFKA-9992: EmbeddedKafkaCluster.deleteTopicAndWait not working with kafka_2.13

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


   test this please


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