----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23339/#review48822 -----------------------------------------------------------
Thanks for the patch. A few more comments below. clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java <https://reviews.apache.org/r/23339/#comment85560> We need to pass in the createTopic flag in this constructor too. In the producer, we will then set createTopic to true and the in the consumer, we will set it to true. clients/src/main/java/org/apache/kafka/common/requests/MetadataRequest.java <https://reviews.apache.org/r/23339/#comment85558> Yes, I agree with you that keeping the V0 constructor is a bit confusing and doesn't provide any value. Could you remove this and the V0 constructor OffsetCommitRequest as well? clients/src/main/java/org/apache/kafka/common/requests/MetadataRequest.java <https://reviews.apache.org/r/23339/#comment85557> Is this needed? core/src/main/scala/kafka/client/ClientUtils.scala <https://reviews.apache.org/r/23339/#comment85559> I think we can just keep the original name and keep the default for createTopic to true to make it backward compatible. core/src/main/scala/kafka/javaapi/TopicMetadataRequest.scala <https://reviews.apache.org/r/23339/#comment85561> To make this backward compatible, we need to keep the current constructor and add a new one with the createTopic flag. - Jun Rao On July 24, 2014, 12:07 a.m., Sriharsha Chintalapani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23339/ > ----------------------------------------------------------- > > (Updated July 24, 2014, 12:07 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1507 > https://issues.apache.org/jira/browse/KAFKA-1507 > > > Repository: kafka > > > Description > ------- > > KAFKA-1507. Using GetOffsetShell against non-existent topic creates the topic > unintentionally. Changes as per Jun's suggestions. > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/clients/NetworkClient.java > d8f9ce663ee24d2b0852c974136741280c39f8f8 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java > 4aa5b01d611631db72df47d50bbe30edb8c478db > clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java > 7517b879866fc5dad5f8d8ad30636da8bbe7784a > clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java > 444e69e7c95d5ffad19896fff0ab15cb4f5c9b4e > clients/src/main/java/org/apache/kafka/common/requests/MetadataRequest.java > b22ca1dce65f665d84c2a980fd82f816e93d9960 > > clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java > df37fc6d8f0db0b8192a948426af603be3444da4 > core/src/main/scala/kafka/api/TopicMetadataRequest.scala > 7dca09ce637a40e125de05703dc42e8b611971ac > core/src/main/scala/kafka/client/ClientUtils.scala > ce7ede3f6d60e756e252257bd8c6fedc21f21e1c > core/src/main/scala/kafka/consumer/ConsumerFetcherManager.scala > b9e2bea7b442a19bcebd1b350d39541a8c9dd068 > core/src/main/scala/kafka/javaapi/TopicMetadataRequest.scala > b0b7be14d494ae8c87f4443b52db69d273c20316 > core/src/main/scala/kafka/server/KafkaApis.scala > fd5f12ee31e78cdcda8e24a0ab3e1740b3928884 > core/src/main/scala/kafka/tools/GetOffsetShell.scala > 9c6064e201eebbcd5b276a0dedd02937439edc94 > core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala > af4783646803e58714770c21f8c3352370f26854 > core/src/main/scala/kafka/tools/SimpleConsumerShell.scala > 36314f412a8281aece2789fd2b74a106b82c57d2 > core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala > 1bf2667f47853585bc33ffb3e28256ec5f24ae84 > core/src/test/scala/unit/kafka/integration/TopicMetadataTest.scala > 35dc071b1056e775326981573c9618d8046e601d > > Diff: https://reviews.apache.org/r/23339/diff/ > > > Testing > ------- > > > Thanks, > > Sriharsha Chintalapani > >
