Hi all, A few comments below.
On Mon, Sep 19, 2016 at 5:49 PM, Jun Rao <j...@confluent.io> wrote: > > 1. I thought at some point you considered to only return offset in > offsetsForTimes instead of offset and timestamp. One benefit of doing that > is that it will make the return type consistent among all three new apis. > It also feels a bit weird that we only return timestamp, but not other > metadata associated with a message. > To me, this API is different enough from the other ones that it seems OK for the return type to be different. And since we are querying by timestamp, it makes sense to get the actual timestamp that matched in the result (as Becket said elsewhere). 2. To be consistent with existing seek apis, would it be better to rename > earliestOffsets() and latestOffsets() to beginningOffsets() and > endOffsets()? > I was actually wondering why we were using `earliest`/`latest` instead of `beginning`/`end`, so I like this suggestion. To ensure the accordance with KIP-45, we have changed the > "earliestOffset()" and "latestOffset()" method to take > Collection<TopicPartition> instead of Set<TopicPartition>. I agree with this change too (predictably since I suggested it :)). Ismael
