You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Chia-Ping Tsai <ch...@apache.org> on 2018/07/01 16:33:22 UTC

[DISCUSS] KIP-331 Add default implementation to close() and configure() for Serializer, Deserializer and Serde

hi folks,

KIP-331 is waiting for any suggestions, feedback and reviews. The main purpose of the KIP-331 is to add empty implementations to the close() and configure() so as to user can write less code to develop custom Serialzier, Deserializer and Serde.

Cheers,
Chia-Ping

Re: [DISCUSS] KIP-331 Add default implementation to close() and configure() for Serializer, Deserializer and Serde

Posted by Saïd Bouras <sa...@gmail.com>.
Hi everyone !

I saw this KIP and I comment the PR, but like *@Chia-Ping* mentioned, it's
better to talk here with community.

Well to summarize, I think that putting the methods *configure* and *close*
default could bring misunderstanding in the approach when implementing
customs serializers and deserializers. Now we know that we have to do some
configurations in theses methods and that it's the default behavior but by
hiding them, developers can think that it's not  "normal" to overrides this
methods.

My idea would be to extends *Serializer* and *Deserializer* interfaces to
specify that we do not need configuration in them, we can call them
*UnConfigureSerializer*/*UnconfigureDeserializer. S*erializers and
deserializers that do nothing in *configure* and *close* methods can
implement them directly and let the other implements the base interface.

That way, developers that begin to work with Kafka can see that we give
them two approach to create their custom serdes.

I don't know if I'm clear, tell me if I don't.
Cheers

On Wed, Jul 4, 2018 at 5:06 PM Chia-Ping Tsai <ch...@apache.org> wrote:

> hi John
>
> > Just really scraping my mind for concerns to investigate... This change
> > won't break source compatibility, but will it affect binary
> compatibility?
> > For example, if I compile my application against Kafka 2.0, for example,
> > and then swap in the Kafka jar containing your change on my classpath at
> > run time, will it still work?
> > I think it should be binary compatible, assuming Oracle didn't do
> anything
> > crazy, since all the method references would be the same. It might we
> worth
> > an experiment, though.
>
> The side effect of this change is IncompatibleClassChangeError caused by
> the conflict of multi default implementations (see
> https://docs.oracle.com/javase/specs/jls/se8/html/jls-13.html#jls-13.5.6).
> But the change won't introduce the such error since there is no default
> impl before.
>
> I will start the Vote at 7/6.
>
> Cheers,
> chia-ping
>
> On 2018/07/03 18:17:28, John Roesler <jo...@confluent.io> wrote:
> > Hi again, Chia-Ping,
> >
> > Thanks! I took a look at it, and it seems like a good change to me.
> >
> > I don't know if it will generate much discussion, so I recommend just
> > letting it steep for another day or two and then moving on to a vote if
> no
> > one else chimes in.
> >
> > Just really scraping my mind for concerns to investigate... This change
> > won't break source compatibility, but will it affect binary
> compatibility?
> > For example, if I compile my application against Kafka 2.0, for example,
> > and then swap in the Kafka jar containing your change on my classpath at
> > run time, will it still work?
> >
> > I think it should be binary compatible, assuming Oracle didn't do
> anything
> > crazy, since all the method references would be the same. It might we
> worth
> > an experiment, though.
> >
> > Thanks,
> > -John
> >
> > On Mon, Jul 2, 2018 at 9:29 PM Chia-Ping Tsai <ch...@apache.org>
> wrote:
> >
> > > > Can you provide a link, please?
> > >
> > > Pardon me. I put the page in the incorrect location.
> > >
> > >
> 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
> > >
> > > Cheers,
> > > Chia-Ping
> > >
> > > On 2018/07/02 19:45:19, John Roesler <jo...@confluent.io> wrote:
> > > > Hi Chia-Ping,
> > > >
> > > > I couldn't find KIP-331 in the list of KIPs (
> > > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals
> > > > ).
> > > >
> > > > Can you provide a link, please?
> > > >
> > > > Thanks,
> > > > -John
> > > >
> > > > On Sun, Jul 1, 2018 at 11:33 AM Chia-Ping Tsai <ch...@apache.org>
> > > wrote:
> > > >
> > > > > hi folks,
> > > > >
> > > > > KIP-331 is waiting for any suggestions, feedback and reviews. The
> main
> > > > > purpose of the KIP-331 is to add empty implementations to the
> close()
> > > and
> > > > > configure() so as to user can write less code to develop custom
> > > Serialzier,
> > > > > Deserializer and Serde.
> > > > >
> > > > > Cheers,
> > > > > Chia-Ping
> > > > >
> > > >
> > >
> >
>
-- 

*Saïd BOURAS*

Re: [DISCUSS] KIP-331 Add default implementation to close() and configure() for Serializer, Deserializer and Serde

Posted by Chia-Ping Tsai <ch...@apache.org>.
hi John

> Just really scraping my mind for concerns to investigate... This change
> won't break source compatibility, but will it affect binary compatibility?
> For example, if I compile my application against Kafka 2.0, for example,
> and then swap in the Kafka jar containing your change on my classpath at
> run time, will it still work?
> I think it should be binary compatible, assuming Oracle didn't do anything
> crazy, since all the method references would be the same. It might we worth
> an experiment, though.

The side effect of this change is IncompatibleClassChangeError caused by the conflict of multi default implementations (see https://docs.oracle.com/javase/specs/jls/se8/html/jls-13.html#jls-13.5.6). But the change won't introduce the such error since there is no default impl before.

I will start the Vote at 7/6.

Cheers,
chia-ping

On 2018/07/03 18:17:28, John Roesler <jo...@confluent.io> wrote: 
> Hi again, Chia-Ping,
> 
> Thanks! I took a look at it, and it seems like a good change to me.
> 
> I don't know if it will generate much discussion, so I recommend just
> letting it steep for another day or two and then moving on to a vote if no
> one else chimes in.
> 
> Just really scraping my mind for concerns to investigate... This change
> won't break source compatibility, but will it affect binary compatibility?
> For example, if I compile my application against Kafka 2.0, for example,
> and then swap in the Kafka jar containing your change on my classpath at
> run time, will it still work?
> 
> I think it should be binary compatible, assuming Oracle didn't do anything
> crazy, since all the method references would be the same. It might we worth
> an experiment, though.
> 
> Thanks,
> -John
> 
> On Mon, Jul 2, 2018 at 9:29 PM Chia-Ping Tsai <ch...@apache.org> wrote:
> 
> > > Can you provide a link, please?
> >
> > Pardon me. I put the page in the incorrect location.
> >
> > 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
> >
> > Cheers,
> > Chia-Ping
> >
> > On 2018/07/02 19:45:19, John Roesler <jo...@confluent.io> wrote:
> > > Hi Chia-Ping,
> > >
> > > I couldn't find KIP-331 in the list of KIPs (
> > >
> > https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals
> > > ).
> > >
> > > Can you provide a link, please?
> > >
> > > Thanks,
> > > -John
> > >
> > > On Sun, Jul 1, 2018 at 11:33 AM Chia-Ping Tsai <ch...@apache.org>
> > wrote:
> > >
> > > > hi folks,
> > > >
> > > > KIP-331 is waiting for any suggestions, feedback and reviews. The main
> > > > purpose of the KIP-331 is to add empty implementations to the close()
> > and
> > > > configure() so as to user can write less code to develop custom
> > Serialzier,
> > > > Deserializer and Serde.
> > > >
> > > > Cheers,
> > > > Chia-Ping
> > > >
> > >
> >
> 

Re: [DISCUSS] KIP-331 Add default implementation to close() and configure() for Serializer, Deserializer and Serde

Posted by John Roesler <jo...@confluent.io>.
Hi again, Chia-Ping,

Thanks! I took a look at it, and it seems like a good change to me.

I don't know if it will generate much discussion, so I recommend just
letting it steep for another day or two and then moving on to a vote if no
one else chimes in.

Just really scraping my mind for concerns to investigate... This change
won't break source compatibility, but will it affect binary compatibility?
For example, if I compile my application against Kafka 2.0, for example,
and then swap in the Kafka jar containing your change on my classpath at
run time, will it still work?

I think it should be binary compatible, assuming Oracle didn't do anything
crazy, since all the method references would be the same. It might we worth
an experiment, though.

Thanks,
-John

On Mon, Jul 2, 2018 at 9:29 PM Chia-Ping Tsai <ch...@apache.org> wrote:

> > Can you provide a link, please?
>
> Pardon me. I put the page in the incorrect location.
>
> 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
>
> Cheers,
> Chia-Ping
>
> On 2018/07/02 19:45:19, John Roesler <jo...@confluent.io> wrote:
> > Hi Chia-Ping,
> >
> > I couldn't find KIP-331 in the list of KIPs (
> >
> https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals
> > ).
> >
> > Can you provide a link, please?
> >
> > Thanks,
> > -John
> >
> > On Sun, Jul 1, 2018 at 11:33 AM Chia-Ping Tsai <ch...@apache.org>
> wrote:
> >
> > > hi folks,
> > >
> > > KIP-331 is waiting for any suggestions, feedback and reviews. The main
> > > purpose of the KIP-331 is to add empty implementations to the close()
> and
> > > configure() so as to user can write less code to develop custom
> Serialzier,
> > > Deserializer and Serde.
> > >
> > > Cheers,
> > > Chia-Ping
> > >
> >
>

Re: [DISCUSS] KIP-331 Add default implementation to close() and configure() for Serializer, Deserializer and Serde

Posted by Chia-Ping Tsai <ch...@apache.org>.
> Can you provide a link, please?

Pardon me. I put the page in the incorrect location. 
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

Cheers,
Chia-Ping

On 2018/07/02 19:45:19, John Roesler <jo...@confluent.io> wrote: 
> Hi Chia-Ping,
> 
> I couldn't find KIP-331 in the list of KIPs (
> https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals
> ).
> 
> Can you provide a link, please?
> 
> Thanks,
> -John
> 
> On Sun, Jul 1, 2018 at 11:33 AM Chia-Ping Tsai <ch...@apache.org> wrote:
> 
> > hi folks,
> >
> > KIP-331 is waiting for any suggestions, feedback and reviews. The main
> > purpose of the KIP-331 is to add empty implementations to the close() and
> > configure() so as to user can write less code to develop custom Serialzier,
> > Deserializer and Serde.
> >
> > Cheers,
> > Chia-Ping
> >
> 

Re: [DISCUSS] KIP-331 Add default implementation to close() and configure() for Serializer, Deserializer and Serde

Posted by John Roesler <jo...@confluent.io>.
Hi Chia-Ping,

I couldn't find KIP-331 in the list of KIPs (
https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals
).

Can you provide a link, please?

Thanks,
-John

On Sun, Jul 1, 2018 at 11:33 AM Chia-Ping Tsai <ch...@apache.org> wrote:

> hi folks,
>
> KIP-331 is waiting for any suggestions, feedback and reviews. The main
> purpose of the KIP-331 is to add empty implementations to the close() and
> configure() so as to user can write less code to develop custom Serialzier,
> Deserializer and Serde.
>
> Cheers,
> Chia-Ping
>