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> 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) > > 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> 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> >> >> 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> 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> 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> 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> 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> >>>>>> >>>>>> 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> 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 >>>>>> >>>>>> Thank you! >>>>>> >>>>>> Best, >>>>>> Daniyar Yeralin >>>>>> >>>>>>> On May 6, 2019, at 11:59 AM, Daniyar Yeralin (JIRA) <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 >>>>>>> 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/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 >>>>>> ] >>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> This message was sent by Atlassian JIRA >>>>>>> (v7.6.3#76005) >>>>>> >>>>>> >>>> >>>> >>
