Re: Review Request 29912: Patch for KAFKA-1852

2015-02-27 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29912/#review74569 --- Ship it! Minor locking issue noted below. I can take care of that.

Re: Review Request 29912: Patch for KAFKA-1852

2015-02-27 Thread Sriharsha Chintalapani
On Feb. 27, 2015, 8:47 p.m., Joel Koshy wrote: Minor locking issue noted below. I can take care of that. This obviously does not cover the case of committing offsets to a topic that is currently being deleted. I think that can be done in a separate jira. Can you file one? Joel,

Re: Review Request 29912: Patch for KAFKA-1852

2015-02-27 Thread Sriharsha Chintalapani
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29912/ --- (Updated Feb. 27, 2015, 9:50 p.m.) Review request for kafka. Bugs:

Re: Review Request 29912: Patch for KAFKA-1852

2015-02-27 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29912/#review74614 --- Ship it! Ship It! - Joel Koshy On Feb. 27, 2015, 9:50 p.m.,

Re: Review Request 29912: Patch for KAFKA-1852

2015-02-18 Thread Sriharsha Chintalapani
On Feb. 13, 2015, 7:01 p.m., Joel Koshy wrote: core/src/main/scala/kafka/server/OffsetManager.scala, line 215 https://reviews.apache.org/r/29912/diff/3/?file=862699#file862699line215 Minor comment. I think this may be better to pass in to the OffsetManager. We should

Re: Review Request 29912: Patch for KAFKA-1852

2015-02-18 Thread Sriharsha Chintalapani
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29912/ --- (Updated Feb. 18, 2015, 9:13 p.m.) Review request for kafka. Bugs:

Re: Review Request 29912: Patch for KAFKA-1852

2015-02-17 Thread Joel Koshy
On Feb. 13, 2015, 7:01 p.m., Joel Koshy wrote: core/src/main/scala/kafka/server/OffsetManager.scala, line 215 https://reviews.apache.org/r/29912/diff/3/?file=862699#file862699line215 Minor comment. I think this may be better to pass in to the OffsetManager. We should

Re: Review Request 29912: Patch for KAFKA-1852

2015-02-17 Thread Sriharsha Chintalapani
On Feb. 13, 2015, 7:01 p.m., Joel Koshy wrote: core/src/main/scala/kafka/server/OffsetManager.scala, line 215 https://reviews.apache.org/r/29912/diff/3/?file=862699#file862699line215 Minor comment. I think this may be better to pass in to the OffsetManager. We should

Re: Review Request 29912: Patch for KAFKA-1852

2015-02-14 Thread Eric Olander
On Feb. 13, 2015, 6:24 p.m., Eric Olander wrote: core/src/main/scala/kafka/server/KafkaApis.scala, line 303 https://reviews.apache.org/r/29912/diff/3/?file=862697#file862697line303 This code could be done using map and getOrElse on the Option rather than using pattern

Re: Review Request 29912: Patch for KAFKA-1852

2015-02-13 Thread Sriharsha Chintalapani
On Feb. 13, 2015, 6:24 p.m., Eric Olander wrote: core/src/main/scala/kafka/server/KafkaApis.scala, line 303 https://reviews.apache.org/r/29912/diff/3/?file=862697#file862697line303 This code could be done using map and getOrElse on the Option rather than using pattern

Re: Review Request 29912: Patch for KAFKA-1852

2015-02-13 Thread Eric Olander
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29912/#review72405 --- core/src/main/scala/kafka/server/KafkaApis.scala

Re: Review Request 29912: Patch for KAFKA-1852

2015-02-13 Thread Gwen Shapira
On Feb. 13, 2015, 6:24 p.m., Eric Olander wrote: core/src/main/scala/kafka/server/KafkaApis.scala, line 303 https://reviews.apache.org/r/29912/diff/3/?file=862697#file862697line303 This code could be done using map and getOrElse on the Option rather than using pattern

Re: Review Request 29912: Patch for KAFKA-1852

2015-02-13 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29912/#review72413 --- minor comment, looks good otherwise

Re: Review Request 29912: Patch for KAFKA-1852

2015-02-12 Thread Sriharsha Chintalapani
--- 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:

Re: Review Request 29912: Patch for KAFKA-1852

2015-02-12 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29912/#review72166 --- core/src/main/scala/kafka/server/OffsetManager.scala

Re: Review Request 29912: Patch for KAFKA-1852

2015-01-19 Thread Sriharsha Chintalapani
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29912/ --- (Updated Jan. 19, 2015, 6:44 p.m.) Review request for kafka. Bugs:

Review Request 29912: Patch for KAFKA-1852

2015-01-14 Thread Sriharsha Chintalapani
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29912/ --- Review request for kafka. Bugs: KAFKA-1852