----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23339/#review48397 -----------------------------------------------------------
Thanks for the patch and doing the rebasing. Some comments below. clients/src/main/java/org/apache/kafka/common/requests/MetadataRequest.java <https://reviews.apache.org/r/23339/#comment84957> The request/response format for the new clients are defined in Protocol. So, we need to make the change there as well and bump up the version #. Then, we need to change this object accordingly. Se OffsetCommitRequest as an example. core/src/main/scala/kafka/api/TopicMetadataRequest.scala <https://reviews.apache.org/r/23339/#comment84958> Does this need to be a short? Could it be just 1 byte? core/src/main/scala/kafka/client/ClientUtils.scala <https://reviews.apache.org/r/23339/#comment84960> The client should just send request using the protocol of the latest version and it shouldn't need to know the version #. core/src/main/scala/kafka/client/ClientUtils.scala <https://reviews.apache.org/r/23339/#comment84963> Perhaps this method can be named better since it may not just be for the consumer. - Jun Rao On July 22, 2014, 5:27 p.m., Sriharsha Chintalapani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23339/ > ----------------------------------------------------------- > > (Updated July 22, 2014, 5:27 p.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. > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/common/requests/MetadataRequest.java > b22ca1dce65f665d84c2a980fd82f816e93d9960 > 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 > >
