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