Hey Sophie,
Thank you for your input. I think I’d rather finish this KIP as is, and then
open a new one for the Collections (if everyone agrees). I don’t want to extend
the current KIP-466, since most of the work is already done for it.
Meanwhile, I’ll start adding some test cases for this new list serde since this
discussion seems to be approaching its logical end.
Best,
Daniyar Yeralin
> On May 9, 2019, at 1:35 PM, Sophie Blee-Goldman <sop...@confluent.io> wrote:
>
> Good point about serdes for other Collections. On the one hand I'd guess
> that non-List Collections are probably relatively rare in practice (if
> anyone disagrees please correct me!) but on the other hand, a) even if just
> a small number of people benefit I think it's worth the extra effort and b)
> if we do end up needing/wanting them in the future it would save us a KIP
> to just add them now. Personally I feel it would make sense to expand the
> scope of this KIP a bit to include all Collections as a logical unit, but
> the ROI could be low..
>
> (I know of at least one instance in the unit tests where a Set serde could
> be useful, and there may be more)
>
> On Thu, May 9, 2019 at 7:27 AM Development <d...@yeralin.net> wrote:
>
>> Hey,
>>
>> I don’t see any replies. Seems like this proposal can be finalized and
>> called for a vote?
>>
>> Also I’ve been thinking. Do we need more serdes for other Collections?
>> Like queue or set for example
>>
>> Best,
>> Daniyar Yeralin
>>
>>> On May 8, 2019, at 2:28 PM, John Roesler <j...@confluent.io> wrote:
>>>
>>> Hi Daniyar,
>>>
>>> No worries about the procedural stuff. Prior experience with KIPs is
>>> not required :)
>>>
>>> I was just trying to help you propose this stuff in a way that the
>>> others will find easy to review.
>>>
>>> Thanks for updating the KIP. Thanks to the others for helping out with
>>> the syntax.
>>>
>>> Given these updates, I'm curious if anyone else has feedback about
>>> this proposal. Personally, I think it sounds fine!
>>>
>>> -John
>>>
>>> On Wed, May 8, 2019 at 1:01 PM Development <d...@yeralin.net> wrote:
>>>>
>>>> Hey,
>>>>
>>>> That worked! I certainly lack Java generics knowledge. Thanks for the
>> snippet. I’ll update KIP again.
>>>>
>>>> Best,
>>>> Daniyar Yeralin
>>>>
>>>>> On May 8, 2019, at 1:39 PM, Chris Egerton <chr...@confluent.io> wrote:
>>>>>
>>>>> Hi Daniyar,
>>>>>
>>>>> I think you may want to tweak your syntax a little:
>>>>>
>>>>> public static <T> Serde<List<T>> List(Serde<T> innerSerde) {
>>>>> return new ListSerde<T>(innerSerde);
>>>>> }
>>>>>
>>>>> Does that work?
>>>>>
>>>>> Cheers,
>>>>>
>>>>> Chris
>>>>>
>>>>> On Wed, May 8, 2019 at 10:29 AM Development <d...@yeralin.net <mailto:
>> d...@yeralin.net>> wrote:
>>>>> Hi John,
>>>>>
>>>>> I updated JIRA and KIP.
>>>>>
>>>>> I didn’t know about the process, and created PR before I knew about
>> KIPs :)
>>>>>
>>>>> As per static declaration, I don’t think Java allows that:
>>>>>
>>>>>
>>>>> Best,
>>>>> Daniyar Yeralin
>>>>>
>>>>>> On May 7, 2019, at 2:22 PM, John Roesler <j...@confluent.io <mailto:
>> j...@confluent.io>> wrote:
>>>>>>
>>>>>> Thanks for that update. Do you mind making changes primarily on the
>>>>>> KIP document ? (
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-466%3A+Add+support+for+List%3CT%3E+serialization+and+deserialization
>> <
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-466%3A+Add+support+for+List%3CT%3E+serialization+and+deserialization
>>> )
>>>>>>
>>>>>> This is the design document that we have to agree on and vote for, the
>>>>>> PR comes later. It can be nice to have an implementation to look at,
>>>>>> but the KIP is the main artifact for this discussion.
>>>>>>
>>>>>> With this in mind, it will help get more reviewers to look at it if
>>>>>> you can tidy up the KIP document so that it stands on its own. People
>>>>>> shouldn't have to look at any other document to understand the
>>>>>> motivation of the proposal, and they shouldn't have to look at a PR to
>>>>>> see what the public API will look like. If it helps, you can take a
>>>>>> look at some other recent KIPs.
>>>>>>
>>>>>> Given that the list serde needs an inner serde, I agree you can't have
>>>>>> a zero-argument static factory method for it, but it seems you could
>>>>>> still have a static method:
>>>>>> `public static Serde<List<T>> List(Serde<T> innerSerde)`.
>>>>>>
>>>>>> Thoughts?
>>>>>>
>>>>>> On Tue, May 7, 2019 at 12:18 PM Development <d...@yeralin.net <mailto:
>> d...@yeralin.net>> wrote:
>>>>>>>
>>>>>>> Absolutely agree. Already pushed another commit to remove comparator
>> argument: https://github.com/apache/kafka/pull/6592 <
>> https://github.com/apache/kafka/pull/6592> <
>> https://github.com/apache/kafka/pull/6592 <
>> https://github.com/apache/kafka/pull/6592>>
>>>>>>>
>>>>>>> Thank you for your input John! I really appreciate it.
>>>>>>>
>>>>>>> What about this point I made:
>>>>>>>
>>>>>>> 1. Since type for List serde needs to be declared before hand, I
>> could not create a static method for List Serde under
>> org.apache.kafka.common.serialization.Serdes. I addressed it in the KIP:
>>>>>>> P.S. Static method corresponding to ListSerde under
>> org.apache.kafka.common.serialization.Serdes (something like static public
>> Serde<List<T>> List() {...} inorg.apache.kafka.common.serialization.Serdes)
>> class cannot be added because type needs to be defined beforehand. That's
>> why one needs to create List Serde in the following fashion:
>>>>>>> new Serdes.ListSerde<String>(Serdes.String(),
>> Comparator.comparing(String::length));
>>>>>>> (can possibly be simplified by declaring import static
>> org.apache.kafka.common.serialization.Serdes.ListSerde)
>>>>>>>
>>>>>>>> On May 7, 2019, at 11:50 AM, John Roesler <j...@confluent.io
>> <mailto:j...@confluent.io>> wrote:
>>>>>>>>
>>>>>>>> Thanks for the reply Daniyar,
>>>>>>>>
>>>>>>>> That makes much more sense! I thought I must be missing something,
>> but I
>>>>>>>> couldn't for the life of me figure it out.
>>>>>>>>
>>>>>>>> What do you think about just taking an argument, instead of for a
>>>>>>>> Comparator, for the Serde of the inner type? That way, the user can
>> control
>>>>>>>> how exactly the inner data gets serialized, while also bounding the
>> generic
>>>>>>>> parameter properly. As for the order, since the list is already in a
>>>>>>>> specific order, which the user themselves controls, it doesn't seem
>>>>>>>> strictly necessary to offer an option to sort the data during
>> serialization.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> -John
>>>>>>>>
>>>>>>>> On Mon, May 6, 2019 at 8:47 PM Development <d...@yeralin.net
>> <mailto:d...@yeralin.net>> wrote:
>>>>>>>>
>>>>>>>>> Hi John,
>>>>>>>>>
>>>>>>>>> I’m really sorry for the confusion. I cloned that JIRA ticket from
>> an old
>>>>>>>>> one about introducing UUID Serde, and I guess was too hasty while
>> editing
>>>>>>>>> the copy to notice the mistake. Just edited the ticket. Sorry for
>> any
>>>>>>>>> inconvenience .
>>>>>>>>>
>>>>>>>>> As per comparator, I agree. Let’s make user be responsible for
>>>>>>>>> implementing comparable interface. I was just thinking to make the
>> serde a
>>>>>>>>> little more flexible (i.e. let user decide in which order records
>> is going
>>>>>>>>> to be inserted into a change log topic).
>>>>>>>>>
>>>>>>>>> Thank you!
>>>>>>>>>
>>>>>>>>> Best,
>>>>>>>>> Daniyar Yeralin
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> On May 6, 2019, at 5:37 PM, John Roesler <j...@confluent.io
>> <mailto:j...@confluent.io>> wrote:
>>>>>>>>>>
>>>>>>>>>> Hi Daniyar,
>>>>>>>>>>
>>>>>>>>>> Thanks for the proposal!
>>>>>>>>>>
>>>>>>>>>> If I understand the point about the comparator, is it just to
>> capture the
>>>>>>>>>> generic type parameter? If so, then anything that implements a
>> known
>>>>>>>>>> interface would work just as well, right? I've been considering
>> adding
>>>>>>>>>> something like the Jackson TypeReference (or similar classes in
>> many
>>>>>>>>> other
>>>>>>>>>> projects). Would this be a good time to do it?
>>>>>>>>>>
>>>>>>>>>> Note that it's not necessary to actually require that the
>> captured type
>>>>>>>>> is
>>>>>>>>>> Comparable (as this proposal currently does), it's just a way to
>> make
>>>>>>>>> sure
>>>>>>>>>> there is some method that makes use of the generic type
>> parameter, to
>>>>>>>>> force
>>>>>>>>>> the compiler to capture the type.
>>>>>>>>>>
>>>>>>>>>> Just to make sure I understand the motivation... You expressed a
>> desire
>>>>>>>>> to
>>>>>>>>>> be able to serialize UUIDs, which I didn't follow, since there is
>> a
>>>>>>>>>> built-in UUID serde:
>> org.apache.kafka.common.serialization.Serdes#UUID,
>>>>>>>>> and
>>>>>>>>>> also, a UUID isn't a List. Did you mean that you need to use
>> *lists of*
>>>>>>>>>> UUIDs?
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> -John
>>>>>>>>>>
>>>>>>>>>> On Mon, May 6, 2019 at 11:49 AM Development <d...@yeralin.net
>> <mailto:d...@yeralin.net>> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hello,
>>>>>>>>>>>
>>>>>>>>>>> Starting a discussion for KIP-466 adding support for List Serde.
>> PR is
>>>>>>>>>>> created under https://github.com/apache/kafka/pull/6592 <
>> https://github.com/apache/kafka/pull/6592> <
>>>>>>>>>>> https://github.com/apache/kafka/pull/6592 <
>> https://github.com/apache/kafka/pull/6592>>
>>>>>>>>>>>
>>>>>>>>>>> There are two topics I would like to discuss:
>>>>>>>>>>> 1. Since type for List serve needs to be declared before hand, I
>> could
>>>>>>>>> not
>>>>>>>>>>> create a static method for List Serde under
>>>>>>>>>>> org.apache.kafka.common.serialization.Serdes. I addressed it in
>> the KIP:
>>>>>>>>>>> P.S. Static method corresponding to ListSerde under
>>>>>>>>>>> org.apache.kafka.common.serialization.Serdes (something like
>> static
>>>>>>>>> public
>>>>>>>>>>> Serde<List<T>> List() {...}
>>>>>>>>> inorg.apache.kafka.common.serialization.Serdes)
>>>>>>>>>>> class cannot be added because type needs to be defined
>> beforehand.
>>>>>>>>> That's
>>>>>>>>>>> why one needs to create List Serde in the following fashion:
>>>>>>>>>>> new Serdes.ListSerde<String>(Serdes.String(),
>>>>>>>>>>> Comparator.comparing(String::length));
>>>>>>>>>>> (can possibly be simplified by declaring import static
>>>>>>>>>>> org.apache.kafka.common.serialization.Serdes.ListSerde)
>>>>>>>>>>>
>>>>>>>>>>> 2. @miguno Michael G. Noll <https://github.com/miguno <
>> https://github.com/miguno>> is questioning
>>>>>>>>>>> whether I need to pass a comparator to ListDeserializer. This
>> certainly
>>>>>>>>> is
>>>>>>>>>>> not required. Feel free to add your input:
>>>>>>>>>>> https://github.com/apache/kafka/pull/6592#discussion_r281152067
>> <https://github.com/apache/kafka/pull/6592#discussion_r281152067>
>>>>>>>>>>>
>>>>>>>>>>> Thank you!
>>>>>>>>>>>
>>>>>>>>>>> Best,
>>>>>>>>>>> Daniyar Yeralin
>>>>>>>>>>>
>>>>>>>>>>>> On May 6, 2019, at 11:59 AM, Daniyar Yeralin (JIRA) <
>> j...@apache.org <mailto:j...@apache.org>>
>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Daniyar Yeralin created KAFKA-8326:
>>>>>>>>>>>> --------------------------------------
>>>>>>>>>>>>
>>>>>>>>>>>> Summary: Add List<T> Serde
>>>>>>>>>>>> Key: KAFKA-8326
>>>>>>>>>>>> URL:
>> https://issues.apache.org/jira/browse/KAFKA-8326 <
>> https://issues.apache.org/jira/browse/KAFKA-8326>
>>>>>>>>>>>> Project: Kafka
>>>>>>>>>>>> Issue Type: Improvement
>>>>>>>>>>>> Components: clients, streams
>>>>>>>>>>>> Reporter: Daniyar Yeralin
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> I propose adding serializers and deserializers for the
>> java.util.List
>>>>>>>>>>> class.
>>>>>>>>>>>>
>>>>>>>>>>>> I have many use cases where I want to set the key of a Kafka
>> message to
>>>>>>>>>>> be a UUID. Currently, I need to turn UUIDs into strings or byte
>> arrays
>>>>>>>>> and
>>>>>>>>>>> use their associated Serdes, but it would be more convenient to
>>>>>>>>> serialize
>>>>>>>>>>> and deserialize UUIDs directly.
>>>>>>>>>>>>
>>>>>>>>>>>> I believe there are many use cases where one would want to have
>> a List
>>>>>>>>>>> serde. Ex. [
>>>>>>>>>>>
>>>>>>>>>
>> https://stackoverflow.com/questions/41427174/aggregate-java-objects-in-a-list-with-kafka-streams-dsl-windows
>> <
>> https://stackoverflow.com/questions/41427174/aggregate-java-objects-in-a-list-with-kafka-streams-dsl-windows
>>>
>>>>>>>>> ],
>>>>>>>>>>> [
>>>>>>>>>>>
>>>>>>>>>
>> https://stackoverflow.com/questions/46365884/issue-with-arraylist-serde-in-kafka-streams-api
>> <
>> https://stackoverflow.com/questions/46365884/issue-with-arraylist-serde-in-kafka-streams-api
>>>
>>>>>>>>>>> ]
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> KIP Link: [
>>>>>>>>>>>
>>>>>>>>>
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-466%3A+Add+support+for+List%3CT%3E+serialization+and+deserialization
>> <
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-466%3A+Add+support+for+List%3CT%3E+serialization+and+deserialization
>>>
>>>>>>>>>>> ]
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> --
>>>>>>>>>>>> This message was sent by Atlassian JIRA
>>>>>>>>>>>> (v7.6.3#76005)
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>
>>>>
>>
>>
