> On Nov. 10, 2014, 7:50 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/consumer/ConsumerConnector.scala, lines 76-80
> > <https://reviews.apache.org/r/27684/diff/2/?file=755292#file755292line76>
> >
> > We will also need to change the interface in ConsumerConnector from
> >
> > def commitOffsets(retryOnFailure: Boolean = true)
> >
> > back to
> >
> > def commitOffsets
> >
> > In ZookeeperConsumerconnector, we can make the following method private
> >
> > def commitOffsets(retryOnFailure: Boolean = true)
> >
> > Another question, will scala compiler be confused with 2 methods, one
> > w/o parenthsis and one with 1 parameter having a default? Could you try
> > compiling the code on all scala versions?
>
> Manikumar Reddy O wrote:
> Currently below classes uses the new method commitOffsets(true).
>
> kafka/javaapi/consumer/ZookeeperConsumerConnector.scala
> kafka/tools/TestEndToEndLatency.scala
>
> If we are changing the interface, then we need to change the above
> classes
> also.
>
> If we are not fixing this on trunk, then same problem will come in 0.8.3.
> How to handle this?
>
> 2 methods, one w/o parenthsis and one with 1 parameter is getting
> compiled on
> all scala versions.
Thanks for the explanation. There is actually a bit of inconsistency introduced
in this patch.
In kafka.javaapi.consumer.ZookeeperConsumerConnector, commitOffsets() is
implemented as the following.
def commitOffsets() {
underlying.commitOffsets()
}
This actually calls underlying.commitOffsets(isAutoCommit: Boolean = true) with
a default value of true. However, ConsumerConnector.commitOffset is implemented
as the following which sets isAutoCommit to false.
def commitOffsets { commitOffsets(false) }
So, we should use true in the above.
Another thing that I was thinking is that it's going to be a bit confusing if
we have the following scala apis.
def commitOffsets(retryOnFailure: Boolean = true)
def commitOffsets
So, if you do commitOffset it calls the second one and if you do
commitOffset(), you actually call the first one. However, the expectation is
probably the same method will be called in both cases. Would it be better if we
get rid of the default like the following? Then, it's clear which method will
be called.
def commitOffsets(retryOnFailure: Boolean)
def commitOffsets
- Jun
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27684/#review60652
-----------------------------------------------------------
On Nov. 8, 2014, 6:20 a.m., Manikumar Reddy O wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27684/
> -----------------------------------------------------------
>
> (Updated Nov. 8, 2014, 6:20 a.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1743
> https://issues.apache.org/jira/browse/KAFKA-1743
>
>
> Repository: kafka
>
>
> Description
> -------
>
> def commitOffsets method added to make ConsumerConnector backward compatible
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/consumer/ConsumerConnector.scala
> 07677c1c26768ef9c9032626180d0015f12cb0e0
>
> Diff: https://reviews.apache.org/r/27684/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Manikumar Reddy O
>
>
