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)
>>>>>>>>> 
>>>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>> 
>>> 
>> 

Reply via email to