> On Sept. 24, 2014, 5:13 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/tools/OffsetClient.scala, line 133 > > <https://reviews.apache.org/r/25944/diff/3/?file=703087#file703087line133> > > > > Should we just use ClientUtils.fetchTopicMetadata?
Made ClientUtils.fetchTopicMetadata use this. > On Sept. 24, 2014, 5:13 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/tools/OffsetClient.scala, line 162 > > <https://reviews.apache.org/r/25944/diff/3/?file=703087#file703087line162> > > > > Better to name this fetchOffsets > > > > Also, it seems annoying for a user to always have to pass in a consumer > > config. Should we just expose the timeouts, etc through this API and > > provide defaults as well? Alternatively we could provide an OffsetClient > > class that takes these parameters once through its constructor. One benefit > > of doing that is it becomes possible to maintain a persistent offset > > manager channel - with this for every ofset fetch/commit the consumer (or > > tools) have to spawn connections to query for metadata then possibly topic > > metadata and then the offset fetch or commit. > > > > Also, consider making a separate OffsetClientConfig class. > > > > Also, see comment below for compatibility with Java. Not sure if the > > response can be easily extracted from the option in Java code Made a seperate OffsetClientConfig class to handle the config properties > On Sept. 24, 2014, 5:13 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/tools/OffsetClient.scala, line 176 > > <https://reviews.apache.org/r/25944/diff/3/?file=703087#file703087line176> > > > > flatMap { case(topic, partitionsMetadata) => > > ... > > } Could not understand this, because topicsMetadata is a sequence of topicsMetadata. Each entry topicsMetadata contains a topic and a sequence of partitionMetadata. - Mayuresh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25944/#review54329 ----------------------------------------------------------- On Sept. 23, 2014, 5:48 p.m., Mayuresh Gharat wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25944/ > ----------------------------------------------------------- > > (Updated Sept. 23, 2014, 5:48 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1013 > https://issues.apache.org/jira/browse/KAFKA-1013 > > > Repository: kafka > > > Description > ------- > > OffsetCLient Tool API. ImportZkOffsets and ExportZkOffsets replaced by > ImportOffsets and ExportOffsets > > > Modified the comments in the headers > > > Corrected a value > > > Diffs > ----- > > config/consumer.properties 83847de30d10b6e78bb8de28e0bb925d7c0e6ca2 > core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala > fbc680fde21b02f11285a4f4b442987356abd17b > core/src/main/scala/kafka/tools/ConfigConstants.scala PRE-CREATION > core/src/main/scala/kafka/tools/ExportOffsets.scala PRE-CREATION > core/src/main/scala/kafka/tools/ImportOffsets.scala PRE-CREATION > core/src/main/scala/kafka/tools/OffsetClient.scala PRE-CREATION > > Diff: https://reviews.apache.org/r/25944/diff/ > > > Testing > ------- > > > Thanks, > > Mayuresh Gharat > >
