kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jason Gustafson <ja...@confluent.io>
Subject Re: [VOTE] KIP-145: Expose Record Headers in Kafka Connect
Date Tue, 23 Jan 2018 20:03:08 GMT
Hey Randall,

It seemed a bit cleaner to me, but I'm not sure if whether there is any
advantage to keeping the interfaces decoupled. I'd probably suggest going
for the simpler API unless you can think of a good reason not to.

-Jason

On Tue, Jan 23, 2018 at 8:47 AM, Randall Hauch <rhauch@gmail.com> wrote:

> I mostly just followed the pattern of the Converter methods, which also
> take the individual components. Really, the only advantage of the current
> approach is that the HeaderConverter implementations are a bit more
> decoupled as they are not aware of the Header/Headers API nor the
> implementation classes, and they don't instantiate the Header instance.
> Instead, the runtime is entirely responsible for that.
>
> However, using the Header interface directly in the method parameters is
> certainly a bit cleaner from an API perspective. I'm open to this
> suggestion if you or anyone else prefers it. It'd be a minor change at this
> point.
>
> Randall
>
> On Mon, Jan 22, 2018 at 7:30 PM, Jason Gustafson <jason@confluent.io>
> wrote:
>
> > +1 (binding)
> >
> > Just one minor comment. It seems a little surprising that HeaderConverter
> > does not use the Header interface. I expected something like this:
> >
> > Header toConnectHeader(String topic, String headerKey, byte[] value);
> > byte[] fromConnectHeader(String topic, Header header);
> >
> > Was there a reason not to do it this way?
> >
> > Thanks,
> > Jason
> >
> > On Mon, Jan 22, 2018 at 4:45 PM, Ted Yu <yuzhihong@gmail.com> wrote:
> >
> > > +1
> > >
> > > On Mon, Jan 22, 2018 at 2:48 PM, Gwen Shapira <gwen@confluent.io>
> wrote:
> > >
> > > > +1 (binding)
> > > >
> > > > This is going to be HUGE! Thank you Randall.
> > > >
> > > > On Mon, Jan 22, 2018 at 1:18 PM Konstantine Karantasis <
> > > > konstantine@confluent.io> wrote:
> > > >
> > > > > Great addition!
> > > > >
> > > > > +1 (non-binding)
> > > > >
> > > > > Konstantine
> > > > >
> > > > > On Sun, Jan 21, 2018 at 7:26 PM, Ewen Cheslack-Postava <
> > > > ewen@confluent.io>
> > > > > wrote:
> > > > >
> > > > > > +1 (binding)
> > > > > >
> > > > > > Thanks for the work on this -- not a small upgrade to the Connect
> > > APIs!
> > > > > >
> > > > > > -Ewen
> > > > > >
> > > > > > On Fri, Jan 19, 2018 at 3:37 PM, Randall Hauch <rhauch@gmail.com
> >
> > > > wrote:
> > > > > >
> > > > > > > Hi everyone,
> > > > > > >
> > > > > > > I'd like to start the voting on this KIP to add support
for
> > headers
> > > > in
> > > > > > > Connect.:
> > > > > > >
> > > > > > > *https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > > 145+-+Expose+Record+Headers+in+Kafka+Connect
> > > > > > > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > > > > 145+-+Expose+Record+Headers+in+Kafka+Connect>*
> > > > > > >
> > > > > > > This does add a fair number of interfaces to our public
API,
> and
> > > > > defines
> > > > > > > some behavioral changes as well.
> > > > > > >
> > > > > > > Thanks! Your feedback is highly appreciated.
> > > > > > >
> > > > > > > Randall
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message