kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Neha Narkhede" <neha.narkh...@gmail.com>
Subject Re: Review Request 28121: Patch for KAFKA-1780
Date Tue, 25 Nov 2014 00:26:14 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28121/#review62909
-----------------------------------------------------------



core/src/main/scala/kafka/consumer/ConsumerIterator.scala
<https://reviews.apache.org/r/28121/#comment105073>

    Since we're changing the behavior of ConsumerIterator, let's document it here. For example,
hasNext is now unblocking.



core/src/main/scala/kafka/utils/IteratorTemplate.scala
<https://reviews.apache.org/r/28121/#comment105076>

    The change is larger than I expected it to be. What is the reason we couldn't change the
peek() implementation and add poll to the IteratorTemplate? Since IteratorTemplate is not
used elsewhere, it may be an ok change. 
    
    But I guess you must've thought about this and chose not to do it this way. Mind explaining
the reasoning?
    
    I also think this is the right way to do things. However, weighing that with the fact
that we are making a somewhat large change to an API that is going away, how bad is implementing
peek by passing in a consumer timeout of 0?


- Neha Narkhede


On Nov. 17, 2014, 5:41 p.m., Ewen Cheslack-Postava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28121/
> -----------------------------------------------------------
> 
> (Updated Nov. 17, 2014, 5:41 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1780
>     https://issues.apache.org/jira/browse/KAFKA-1780
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1780 Add peek and poll methods to ConsumerIterator.
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/consumer/ConsumerIterator.scala 78fbf75651583e390258af2d9f09df6911a97b59

>   core/src/main/scala/kafka/utils/IteratorTemplate.scala fd952f3ec0f04a3ba639c02779634265489fd186

>   core/src/test/scala/unit/kafka/consumer/ConsumerIteratorTest.scala c0355cc0135c6af2e346b4715659353a31723b86

>   core/src/test/scala/unit/kafka/utils/IteratorTemplateTest.scala 46a4e899ef293c56a931bfa5bcf9a07d07ec5792

> 
> Diff: https://reviews.apache.org/r/28121/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ewen Cheslack-Postava
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message