Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Kafka >> mail # dev >> Review Request 24006: Patch for KAFKA-1420


Copy link to this message
-
Re: Review Request 24006: Patch for KAFKA-1420

This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24006/#review49049

core/src/test/scala/unit/kafka/admin/AdminTest.scala
<https://reviews.apache.org/r/24006/#comment85879>

    Could you group kafka imports together before java/scala/other-libs imports?

core/src/test/scala/unit/kafka/admin/AdminTest.scala
<https://reviews.apache.org/r/24006/#comment85878>

    Could we use
    
    val numPartitions = 12
    val replicationFactor = 3
    
    and then create expectedReplicaAssignment and leaderForPartitionMap based on these two variables, and re-use them here?

core/src/test/scala/unit/kafka/admin/AdminTest.scala
<https://reviews.apache.org/r/24006/#comment85880>

    expectedReplicaAssignment seems not used any more.

core/src/test/scala/unit/kafka/admin/AdminTest.scala
<https://reviews.apache.org/r/24006/#comment85888>

    Could you add a comment here for bouncing server 1?

core/src/test/scala/unit/kafka/admin/AdminTest.scala
<https://reviews.apache.org/r/24006/#comment85890>

    Is there a specific reason we want to use 10 seconds instead of default 5 seconds?

core/src/test/scala/unit/kafka/admin/AdminTest.scala
<https://reviews.apache.org/r/24006/#comment85889>

    Is this println intended?

core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala
<https://reviews.apache.org/r/24006/#comment85891>

    Do we still need expectedReplicaAssignment?

core/src/test/scala/unit/kafka/utils/TestUtils.scala
<https://reviews.apache.org/r/24006/#comment85892>

    Could we just set the default value of configs parameter to null, instead of creating a separate function?
- Guozhang Wang
On July 28, 2014, 8:52 p.m., Jonathan Natkins wrote: