> On March 25, 2015, 9:51 p.m., Guozhang Wang wrote: > > core/src/test/scala/unit/kafka/utils/TestUtils.scala, lines 761-773 > > <https://reviews.apache.org/r/31606/diff/2/?file=881937#file881937line761> > > > > Compression code is no longer used anymore, which seems not correct? > > Flutra Osmani wrote: > The compression.codec is now set on sendMessages()
Isn't this function sendMessages()? Now when we call createProducer() we will not pass in the props anymore and hence it will always use default values, is that OK? > On March 25, 2015, 9:51 p.m., Guozhang Wang wrote: > > core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala, > > lines 116-134 > > <https://reviews.apache.org/r/31606/diff/2/?file=881935#file881935line116> > > > > Can we get rid of this function since it is only called once, and hence > > we can just put the logic there? > > Flutra Osmani wrote: > Same as above: > In kafka.javaapi.consumer.ZookeeperConsumerConnectorTest.scala is the > intention to test the Java API integration, or the actual functionality of > getMessages in this case? I was not thinking about replacing it with getMessages, but just put this function logic to the place where it is triggered as it is only called once. I think it is OK to just keep it as is now. > On March 25, 2015, 9:51 p.m., Guozhang Wang wrote: > > core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala, > > lines 85-106 > > <https://reviews.apache.org/r/31606/diff/2/?file=881935#file881935line85> > > > > Can we use TestUtils.sendMessages to replace this function? > > Flutra Osmani wrote: > My understanding was that we wanted to test > kafka.javaapi.producer.Producer here (not necessarily > kafka.producer.Producer). > > In kafka.javaapi.consumer.ZookeeperConsumerConnectorTest.scala is the > intention to test the Java API integration, or the actual functionality of > sendMessages? Hmm that is a good point. I think we can just keep the java producer here, but with a bit refactoring: for example we can get rid of the compression type as it is not used anywhere. - Guozhang ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31606/#review77804 ----------------------------------------------------------- On March 26, 2015, 7:21 a.m., Flutra Osmani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31606/ > ----------------------------------------------------------- > > (Updated March 26, 2015, 7:21 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1416 > https://issues.apache.org/jira/browse/KAFKA-1416 > > > Repository: kafka > > > Description > ------- > > Unified get and send messages in TestUtils.scala and its users > > > Diffs > ----- > > > core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala > 155fd046c9036b5b8b107bd21a8c6492c14644ea > core/src/test/scala/unit/kafka/integration/FetcherTest.scala > 3093e459935ecf8e5b34fca34a422674562a7034 > core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala > 8342cae564ebc39fe74a512343a4523072ca205a > > core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala > 3d0fc9deda2d3a39f2618a5be3edd98cd935ffbb > core/src/test/scala/unit/kafka/metrics/MetricsTest.scala > 0f58ad8e698e3c0ec76c510bd5f76912a992209c > core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala > 92d6b2c672f74cdd526f2e98da8f7fb3696a88e3 > core/src/test/scala/unit/kafka/utils/TestUtils.scala > 1682a77362123449de6bb1d54a55887409990a24 > > Diff: https://reviews.apache.org/r/31606/diff/ > > > Testing > ------- > > > Thanks, > > Flutra Osmani > >
