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