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