kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chia-Ping Tsai <chia7...@apache.org>
Subject Re: [DISCUSSION] KIP-336: Consolidate ExtendedSerializer/Serializer and ExtendedDeserializer/Deserializer
Date Fri, 20 Jul 2018 12:05:43 GMT
> The approach where all the methods have a default implementation and the
> user chooses to override one of them seems the most appealing to me given
> the current state. 

sounds good. one Q: Which implementation is suitable for serialize()/deserialize()? maybe
just throw exception?

BTW, making all method have a default implementation also disables us from using lambda expressions
to represent a instance of Serializer/Deserializer. We will lose a nice feature from JDK8
:(

Chia-Ping

On 2018/07/17 08:39:32, Ismael Juma <ismael@juma.me.uk> wrote: 
> Hi Viktor,
> 
> The approach where all the methods have a default implementation and the
> user chooses to override one of them seems the most appealing to me given
> the current state. It doesn't seem like we give up much in that case, what
> do you think?
> 
> Ismael
> 
> On Tue, Jul 10, 2018 at 7:15 AM Viktor Somogyi <viktorsomogyi@gmail.com>
> wrote:
> 
> > Hi Ismael,
> >
> > Well, yes. If we care about headers only then you'd need to add a dummy
> > implementation for the 2 parameter method as well. Although it is not
> > ideal, we're using the Serializer interface everywhere and convert it to
> > extended with ensureExtended(serializer) and delegate to the 2 parameter
> > method inside the wrapper which is returned in ensureExtended. Because of
> > backward compatibility we have to keep delegating but I could perhaps add a
> > dummy implementation for the 2 parameter too if you and others think that
> > would be better. In this case though we'd have an interface where all
> > methods are default (given the improvements of KIP-331
> > <
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-331+Add+default+implementation+to+close%28%29+and+configure%28%29+for+Serializer%2C+Deserializer+and+Serde
> > >)
> > and would have to rethink if this interface should be a
> > @FunctionalInterface.
> > I don't really have a context currently on how the 3 parameter method is
> > used, most the code samples I found on github were using the 2 parameter
> > method. I think I found one instance where the 3 parameter one was used but
> > that delegated to the 2 param one :). Have to say though that this research
> > is not representative.
> > All in all I think it wouldn't hurt to provide a default implementation for
> > the 2 param method too but then we have to give up the @FunctionalInterface
> > annotation and we'll end up with an interface with no abstract methods but
> > only defaults.
> > What do you think?
> >
> > Cheers,
> > Viktor
> >
> >
> > On Mon, Jul 9, 2018 at 11:02 AM Ismael Juma <ismael@juma.me.uk> wrote:
> >
> > > Thanks for the KIP. It would be helpful to understand the user experience
> > > for the case where the implementor uses the headers. It seems like it
> > would
> > > require overriding two methods?
> > >
> > > Ismael
> > >
> > > On Mon, Jul 9, 2018 at 1:50 AM Viktor Somogyi <viktorsomogyi@gmail.com>
> > > wrote:
> > >
> > > > Hi folks,
> > > >
> > > > I've published KIP-336 which is about consolidating the
> > > > Serializer/Deserializer interfaces.
> > > >
> > > > Basically the story here is when ExtendedSerializer and
> > > > ExtendedDeserializer were added we still supported Java 7 and therefore
> > > had
> > > > to use compatible constructs which now seem unnecessary since we
> > dropped
> > > > support for Java 7. Now in this KIP I propose a way to deprecate those
> > > > patterns:
> > > >
> > >
> > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=87298242
> > > >
> > > > I'd be happy to receive some feedback about the KIP I published.
> > > >
> > > > Cheers,
> > > > Viktor
> > > >
> > >
> >
> 

Mime
View raw message