Hi,

I was trying to add some test cases for the list serde, and it led me to this 
class `org.apache.kafka.common.serialization.SerializationTest`. I saw that it 
relies on method `org.apache.kafka.common.serialization.serdeFrom(Class<T> 
type)`

Now, I’m not sure how to adapt List<T> serde for this method, since it will be 
a “nested class”. What is the best approach in this case? 

I remember that in Jackson for example, one uses a TypeFactory, and constructs 
“collectionType” of two classes. For example, 
`constructCollectionType(List.class, String.class).getClass()`. I don’t think 
it applies here.

Any ideas?

Best,
Daniyar Yeralin

> On May 9, 2019, at 2:10 PM, Development <d...@yeralin.net> wrote:
> 
> 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)
>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>> 
>>>>>> 
>>>>> 
>>> 
>>> 
> 

Reply via email to