Re: Review Request 31806: Patch for KAFKA-1501

2015-03-08 Thread Eric Olander
/ProducerFailureHandlingTest.scala https://reviews.apache.org/r/31806/#comment122863 This is using the same variable name as the outer loop. - Eric Olander On March 6, 2015, 6:51 p.m., Ewen Cheslack-Postava wrote: --- This is an automatically generated e-mail

Re: Review Request 31366: Patch for KAFKA-1461

2015-02-25 Thread Eric Olander
://reviews.apache.org/r/31366/#comment120684 Again, foreach would be more idomatic, or take advantage of already being in a for loop. - Eric Olander On Feb. 24, 2015, 6:02 p.m., Sriharsha Chintalapani wrote: --- This is an automatically generated e

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 matching

Re: Review Request 29912: Patch for KAFKA-1852

2015-02-13 Thread Eric Olander
== OffsetRequest.LatestTime || timestamp == OffsetRequest.EarliestTime) Seq(0L) else Nil } - Eric Olander On Feb. 13, 2015, 12:46 a.m., Sriharsha Chintalapani wrote: --- This is an automatically generated e-mail. To reply

Re: Review Request 29468: Patch for KAFKA-1805

2015-02-10 Thread Eric Olander
/ProducerRecord.java https://reviews.apache.org/r/29468/#comment117848 return key.equals(that.key) partition.equals(that.partition) topic.equals(that.topic) value.equals(that.value); - Eric Olander On Dec. 30, 2014, 12:37 a.m., Parth Brahmbhatt wrote

Re: Review Request 29831: Patch for KAFKA-1476

2015-02-10 Thread Eric Olander
://reviews.apache.org/r/29831/#comment117844 This can be made a little simpler - getConsumer(zkClient, brokerId).foreach{ consumer = lines 218-225 kept in here } then the case on 226 is no longer needed and the temp variable consumerOpt is eliminated. - Eric Olander

Re: Review Request 30063: Patch for KAFKA-1840

2015-01-31 Thread Eric Olander
://reviews.apache.org/r/30063/#comment115724 Class name should be capitalized. Can this be an object instead of a class? - Eric Olander On Jan. 31, 2015, 2:25 a.m., Jiangjie Qin wrote: --- This is an automatically generated e-mail. To reply

Re: Review Request 30063: Patch for KAFKA-1840

2015-01-31 Thread Eric Olander
= Utils.createObject[MirrorMakerMessageHandler](mirrorMakerMessageHandlerClass) }.getOrElse(new defaultMirrorMakerMessageHandler) - Eric Olander On Jan. 31, 2015, 2:25 a.m., Jiangjie Qin wrote: --- This is an automatically generated e-mail

Re: Review Request 30321: Patch for kafka-1902

2015-01-28 Thread Eric Olander
On Jan. 28, 2015, 2:34 a.m., Eric Olander wrote: core/src/main/scala/kafka/metrics/KafkaMetricsGroup.scala, line 66 https://reviews.apache.org/r/30321/diff/1/?file=836389#file836389line66 val scope = tagsName.map {t = nameBuilder.append(,).append(t)}.orNull Sorry - I have a typo

Re: Review Request 30321: Patch for kafka-1902

2015-01-28 Thread Eric Olander
On Jan. 28, 2015, 2:34 a.m., Eric Olander wrote: core/src/main/scala/kafka/metrics/KafkaMetricsGroup.scala, line 66 https://reviews.apache.org/r/30321/diff/1/?file=836389#file836389line66 val scope = tagsName.map {t = nameBuilder.append(,).append(t)}.orNull Eric Olander wrote

Re: Review Request 28769: Patch for KAFKA-1809

2015-01-27 Thread Eric Olander
in this project :-( core/src/test/scala/unit/kafka/network/SocketServerTest.scala https://reviews.apache.org/r/28769/#comment114607 Scalatest has a helper method for exactly this sort of test: intercept[IOException] { sendRequest(plainSocket, 0, bytes) } - Eric Olander

Re: Review Request 30321: Patch for kafka-1902

2015-01-27 Thread Eric Olander
://reviews.apache.org/r/30321/#comment114778 val scope = tagsName.map {t = nameBuilder.append(,).append(t)}.orNull - Eric Olander On Jan. 27, 2015, 4:16 p.m., Jun Rao wrote: --- This is an automatically generated e-mail. To reply

Re: Review Request 30259: Add static code coverage reporting capability

2015-01-26 Thread Eric Olander
. Probably would be good to add a TODO explaining that once scoverage can process this class the $COVERAGE-OFF$ should be removed. - Eric Olander On Jan. 25, 2015, 8:47 p.m., Ashish Singh wrote: --- This is an automatically generated

[jira] [Commented] (KAFKA-1818) Code cleanup in ReplicationUtils including unit test

2015-01-22 Thread Eric Olander (JIRA)
[ https://issues.apache.org/jira/browse/KAFKA-1818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14288328#comment-14288328 ] Eric Olander commented on KAFKA-1818: - In review: https://reviews.apache.org/r/29840

Re: Review Request 30126: Patch for KAFKA-1845

2015-01-21 Thread Eric Olander
).asInstanceOf[String] then: zkConnect = stringProp(ZkConnectProp) - Eric Olander On Jan. 21, 2015, 5:49 p.m., Andrii Biletskyi wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r

Re: Review Request 29647: Patch for KAFKA-1697

2015-01-19 Thread Eric Olander
/src/main/scala/kafka/cluster/Partition.scala https://reviews.apache.org/r/29647/#comment113039 Again, totally unrelated, but all this needs is: Option(assignedReplicaMap.get(replicaId)) Option.apply() already does what this code is doing. - Eric Olander On Jan. 14, 2015

Re: Review Request 24214: Patch for KAFKA-1374

2015-01-18 Thread Eric Olander
https://reviews.apache.org/r/24214/#comment112888 Could be simplified to just: for (codec - CompressionType.values) yield Array(codec.name) - Eric Olander On Jan. 17, 2015, 6:53 p.m., Manikumar Reddy O wrote

Re: Review Request 29714: Patch for KAFKA-1810

2015-01-16 Thread Eric Olander
of testing: intercept[IPFilterConfigException] { new IPFilter(range, ruleType) } That does the same as the try/catch with the fail() in the try block. - Eric Olander On Jan. 16, 2015, 12:48 a.m., Jeff Holoman wrote

Re: Review Request 25944: Patch for KAFKA-1013

2015-01-13 Thread Eric Olander
at scala.None$.get(Option.scala:322) ... 33 elided Maybe this method can return Option[BlockingChannel] instead of BlockingChannel? - Eric Olander On Jan. 14, 2015, 12:43 a.m., Mayuresh Gharat wrote

Re: Review Request 29668: Patch for KAFKA-1844

2015-01-13 Thread Eric Olander
://reviews.apache.org/r/29668/#comment112129 typo - configuraiton - Eric Olander On Jan. 7, 2015, 8:50 p.m., Sriharsha Chintalapani wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29668

Review Request 29840: Patch for KAFKA-1818

2015-01-12 Thread Eric Olander
84e08557de5acdcf0a98b192feac72836ea359b8 Diff: https://reviews.apache.org/r/29840/diff/ Testing --- Thanks, Eric Olander

Re: Review Request 29231: Patch for KAFKA-1824

2014-12-19 Thread Eric Olander
On Dec. 19, 2014, 2:36 a.m., Eric Olander wrote: core/src/main/scala/kafka/tools/ConsoleProducer.scala, line 269 https://reviews.apache.org/r/29231/diff/1/?file=797000#file797000line269 remove() returns the value assigned to the key being removed, so you could simply do

Re: Review Request 29231: Patch for KAFKA-1824

2014-12-18 Thread Eric Olander
://reviews.apache.org/r/29231/#comment108830 remove() returns the value assigned to the key being removed, so you could simply do: topic = props.remove(topic) instead of the getProperty() and remove() - Eric Olander On Dec. 19, 2014, 1:56 a.m., Gwen Shapira wrote

Re: Review Request 24704: Patch for KAFKA-1499

2014-12-17 Thread Eric Olander
) } If the resulting collection needs to be a java Collection you could import the JavaConversions implicits, or just ignore me. - Eric Olander On Dec. 16, 2014, 5:10 p.m., Manikumar Reddy O wrote: --- This is an automatically

[jira] [Created] (KAFKA-1818) Code cleanup in ReplicationUtils including unit test

2014-12-14 Thread Eric Olander (JIRA)
Eric Olander created KAFKA-1818: --- Summary: Code cleanup in ReplicationUtils including unit test Key: KAFKA-1818 URL: https://issues.apache.org/jira/browse/KAFKA-1818 Project: Kafka Issue Type

[jira] [Updated] (KAFKA-1818) Code cleanup in ReplicationUtils including unit test

2014-12-14 Thread Eric Olander (JIRA)
[ https://issues.apache.org/jira/browse/KAFKA-1818?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Eric Olander updated KAFKA-1818: Status: Patch Available (was: Open) From bb32b9bfbcc5e418cbb0b6b4e9f6aa39d5ce1345 Mon Sep 17 00:00

[jira] [Updated] (KAFKA-1818) Code cleanup in ReplicationUtils including unit test

2014-12-14 Thread Eric Olander (JIRA)
[ https://issues.apache.org/jira/browse/KAFKA-1818?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Eric Olander updated KAFKA-1818: Attachment: 0001-KAFKA-1818-clean-up-code-to-more-idiomatic-scala-usa.patch Code cleanup