----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29912/#review72413 -----------------------------------------------------------
minor comment, looks good otherwise core/src/main/scala/kafka/server/KafkaApis.scala <https://reviews.apache.org/r/29912/#comment118470> (Personally I also prefer the map+getOrElse idiom as it is way more concise. It does take getting used to - I use it but some prefer case matching) core/src/main/scala/kafka/server/OffsetManager.scala <https://reviews.apache.org/r/29912/#comment118473> Minor comment. I think this may be better to pass in to the OffsetManager. We should even use it in loadOffsets to discard offsets that are from topics that have been deleted. We can do that in a separate jira - I don't think our handling for clearing out offsets on a delete topic is done yet - Onur Karaman did it for ZK based offsets but we need a separate jira to delete Kafka-based offsets. - Joel Koshy On Feb. 13, 2015, 12:46 a.m., Sriharsha Chintalapani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29912/ > ----------------------------------------------------------- > > (Updated Feb. 13, 2015, 12:46 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1852 > https://issues.apache.org/jira/browse/KAFKA-1852 > > > Repository: kafka > > > Description > ------- > > KAFKA-1852. OffsetCommitRequest can commit offset on unknown topic. Added > contains method to MetadataCache. > > > Diffs > ----- > > core/src/main/scala/kafka/server/KafkaApis.scala > 6ee7d8819a9ef923f3a65c865a0a3d8ded8845f0 > core/src/main/scala/kafka/server/MetadataCache.scala > 4c70aa7e0157b85de5e24736ebf487239c4571d0 > core/src/main/scala/kafka/server/OffsetManager.scala > 83d52643028c5628057dc0aa29819becfda61fdb > core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala > 5b93239cdc26b5be7696f4e7863adb9fbe5f0ed5 > > Diff: https://reviews.apache.org/r/29912/diff/ > > > Testing > ------- > > > Thanks, > > Sriharsha Chintalapani > >
