Thanks for the KIP Guozhang. > Another reason is that other functions of KafkaConsumers do not have those > overloaded functions to be consistent
I tend to agree with Jason about keeping the existing methods. Your argument does not seem to hold. I just checked the `Consumer` API, and it's mix of overloads: Methods only talking `Collections` subscribe/assign/commitSync/commitAsyn/pause/resume/offsetsForTimes/beginningOffsets/endOffsets Method with overload taking `Collections` or as single value: seek/seekToBeginning/seekToEnd (those are strictly different methods, but they are semantically related) Only talking single value: position/committed/partitionsFor While you are strictly speaking correct, that there is no method with an overload for `Collection` and single value, the API mix seems to suggest that it might actually be worth to have corresponding overloads for all methods instead of sticking to `Collections` only. About the return map: I agree that not containing any entry in the map is better. It's not only consistent with other APIs but it also avoids potential NPEs. -Matthias On 9/10/19 10:04 AM, Jason Gustafson wrote: >> I feel it not worth making committed to have both plurals and singulars. > > Not sure I agree. If we had started with these new APIs from the beginning, > that may have been better, but we already have exposed the singular APIs > and users are depending on them. Not sure it's worth breaking compatibility > just for aesthetics. > > -Jason > > On Tue, Sep 10, 2019 at 9:41 AM Guozhang Wang <wangg...@gmail.com> wrote: > >> Thanks Jason! >> >> On Tue, Sep 10, 2019 at 9:07 AM Jason Gustafson <ja...@confluent.io> >> wrote: >> >>> Hi Guozhang, >>> >>> I think the motivation for the new API makes sense. I've wanted something >>> like this in the past. That said, do you think there is a substantial >>> benefit from deprecating the old API? I can still see it being convenient >>> in some cases and it's no real cost to maintain. >>> >>> >> That's a good question. >> >> Personally I would like to keep a succinct set of APIs out of the box and >> let users who want more syntax sugars to add themselves as extended classes >> for example (KafkaConsumer is not a final class). >> Another reason is that other functions of KafkaConsumers do not have those >> overloaded functions to be consistent, e.g. we do not have a >> subscribe(single-topic), pause/resume(single-topic-partition) or >> seekToBeginning(single-topic-partition). I feel it not worth making >> committed to have both plurals and singulars. >> >> WDYT? >> >> >>> Also, just a minor detail. If the partition has no committed offset, will >>> it be present in the map with a null value? >>> >>> I looked into the admin client's listConsumerGroupOffsets call when >> creating the KIP, and to be consistent with that API my intention is to NOT >> include the entry if a topic-partition does not have committed offsets. >> That said, if we feel returning an entry with null value is better for >> programmability I can also do that (and will update wiki page to clarify as >> well). LMK. >> >> >>> -Jason >>> >>> On Tue, Sep 10, 2019 at 6:09 AM Mickael Maison <mickael.mai...@gmail.com >>> >>> wrote: >>> >>>> +1 (non-binding), thanks Guozhang >>>> >>>> On Tue, Sep 10, 2019 at 1:14 AM Boyang Chen < >> reluctanthero...@gmail.com> >>>> wrote: >>>>> >>>>> Hey Guozhang, >>>>> >>>>> LGTM, +1 (non-binding) >>>>> >>>>> On Mon, Sep 9, 2019 at 5:07 PM Guozhang Wang <wangg...@gmail.com> >>> wrote: >>>>> >>>>>> Hello folks, >>>>>> >>>>>> I've created a new KIP allowing consumer.committed to take a set of >>>>>> partitions instead of just one partition to allow batching effects >> of >>>> such >>>>>> requests (the protocol already allows us to send multiple >> partitions >>>> in one >>>>>> round-trip): >>>>>> >>>>>> >>>>>> >>>> >>> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-520%3A+Add+overloaded+Consumer%23committed+for+batching+partitions >>>>>> >>>>>> Since it is a pretty straight-forward KIP, I'm starting the VOTE >> for >>>> this >>>>>> KIP directly. If there are any suggestions about this proposal, >>> please >>>> feel >>>>>> free to share them in this thread. Thank you! >>>>>> >>>>>> >>>>>> -- Guozhang >>>>>> >>>> >>> >> >> >> -- >> -- Guozhang >> >
signature.asc
Description: OpenPGP digital signature
