You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Yaroslav Tkachenko <sa...@gmail.com> on 2019/03/12 04:01:24 UTC
[DISCUSS] KIP-440: Extend Connect Converter to support headers
Hello,
I'd like to propose a KIP that extends Kafka Connect Converter interface:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-440%3A+Extend+Connect+Converter+to+support+headers
Thanks for considering!
--
Yaroslav Tkachenko
sap1ens.com
Re: [DISCUSS] KIP-440: Extend Connect Converter to support headers
Posted by sa...@gmail.com,
sa...@gmail.com.
Hi Randall,
Sorry, I was away. Just started a vote a few minutes ago.
On 2019/05/06 17:46:23, Randall Hauch <rh...@gmail.com> wrote:
> Have we started a vote on this? I don't see the separate "[VOTE]" thread.
>
> On Mon, Apr 29, 2019 at 6:19 PM Konstantine Karantasis <
> konstantine@confluent.io> wrote:
>
> > Thanks Yaroslav, this KIP LGTM now too!
> >
> > To give some context regarding my previous comment: headers in Connect
> > would probably have followed a similar approach if default methods in
> > interfaces could be used at the time. But we could not have assumed java 8
> > or later yet in the AK version that Connect headers were added, so, I
> > believe, that led to two different converter interfaces.
> >
> > Thanks for the nicely written KIP!
> > Konstantine
> >
> >
> >
> > On Mon, Apr 29, 2019 at 3:39 PM Randall Hauch <rh...@gmail.com> wrote:
> >
> > > Thanks for the update. Yes, IMO this KIP is ready for a vote.
> > >
> > > On Fri, Apr 26, 2019 at 12:15 AM sapiensy@gmail.com <sa...@gmail.com>
> > > wrote:
> > >
> > > > Hi Randall, Konstantine,
> > > >
> > > > I've updated the KIP to reflect the details we discussed here. Let me
> > > know
> > > > if it looks good and we can go to the voting phase.
> > > >
> > > > Thanks!
> > > >
> > > > On 2019/04/22 21:07:31, Randall Hauch <rh...@gmail.com> wrote:
> > > > > I think it would be helpful to clarify this in the KIP, just so that
> > > > > readers are aware that the headers will be the raw header bytes that
> > > are
> > > > > the same as what is in the Kafka record.
> > > > >
> > > > > The alternative I was referring to is exposing the Connect `Headers`
> > > > > interface, which is different.
> > > > >
> > > > > On Mon, Apr 22, 2019 at 1:45 PM sapiensy@gmail.com <
> > sapiensy@gmail.com
> > > >
> > > > > wrote:
> > > > >
> > > > > > Hi Konstantine, Randall,
> > > > > >
> > > > > > As you can see in the updated Converter interface, it always
> > operates
> > > > on
> > > > > > `org.apache.kafka.common.header.Headers`.
> > > > > >
> > > > > > WorkerSinkTask simply uses Kafka message headers and passes them to
> > > the
> > > > > > `toConnectData` method.
> > > > > >
> > > > > > WorkerSourceTask leverages header converter to extract
> > RecordHeaders,
> > > > > > which implements Headers interface. Then RecordHeaders are passed
> > to
> > > > the
> > > > > > `fromConnectData` method.
> > > > > >
> > > > > > So header converter is used as a way to get headers when converting
> > > > data
> > > > > > from internal Connect format to Kafka messages (cause there is no
> > > > other way
> > > > > > to get the headers in this case).
> > > > > >
> > > > > > I can add this to the KIP if it's helpful.
> > > > > >
> > > > > > Randall, what is the alternative approach you're referring to?
> > > > > >
> > > > > > On 2019/04/22 18:09:24, Randall Hauch <rh...@gmail.com> wrote:
> > > > > > > Konstantine raises a good point. Which `Headers` is being
> > > referenced
> > > > in
> > > > > > the
> > > > > > > API? The Connect `org.apache.kafka.connect.header.Headers` would
> > > > > > correspond
> > > > > > > to what was already deserialized by the `HeaderConverter` or what
> > > > will
> > > > > > yet
> > > > > > > be serialized by the `HeaderConverter`. Alternatively, the
> > common `
> > > > > > > org.apache.kafka.common.header.Headers` would correspond to the
> > raw
> > > > > > header
> > > > > > > pairs from the underlying Kafka record.
> > > > > > >
> > > > > > > So, we probably want to be a bit more specific, and also mention
> > > > why. And
> > > > > > > we probably want to mention the other approach in the rejected
> > > > > > alternatives.
> > > > > > >
> > > > > > > Best regards,
> > > > > > >
> > > > > > > Randall
> > > > > > >
> > > > > > > On Mon, Apr 22, 2019 at 11:59 AM Konstantine Karantasis <
> > > > > > > konstantine@confluent.io> wrote:
> > > > > > >
> > > > > > > > Thanks for the KIP Yaroslav!
> > > > > > > >
> > > > > > > > Apologies for the late comment. However, after reading the KIP
> > > it's
> > > > > > still
> > > > > > > > not very clear to me what happens to the existing
> > > > > > > > HeaderConverter interface and what's the expectation for
> > existing
> > > > code
> > > > > > > > implementing this interface out there.
> > > > > > > >
> > > > > > > > Looking at the PR I see that the existing code is leaving the
> > > > existing
> > > > > > > > connect headers conversion unaffected. I'd expect by reading
> > the
> > > > KIP to
> > > > > > > > understand what's the interplay of the current proposal with
> > the
> > > > > > existing
> > > > > > > > implementation of KIP-145 that introduced headers in Connect.
> > > > > > > >
> > > > > > > > Thanks,
> > > > > > > > Konstantine
> > > > > > > >
> > > > > > > > On Mon, Apr 22, 2019 at 9:07 AM Randall Hauch <
> > rhauch@gmail.com>
> > > > > > wrote:
> > > > > > > >
> > > > > > > > > Thanks for updating the KIP. It looks good to me, and since
> > > there
> > > > > > haven't
> > > > > > > > > been any other issue mentioned in this month-long thread,
> > it's
> > > > > > probably
> > > > > > > > > fine to start a vote.
> > > > > > > > >
> > > > > > > > > Best regards,
> > > > > > > > >
> > > > > > > > > Randall
> > > > > > > > >
> > > > > > > > > On Tue, Apr 2, 2019 at 3:12 PM Randall Hauch <
> > rhauch@gmail.com
> > > >
> > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Thanks for the submission, Yaroslav -- and for building on
> > > the
> > > > > > > > suggestion
> > > > > > > > > > of Jeremy C in
> > > > https://issues.apache.org/jira/browse/KAFKA-7273.
> > > > > > This
> > > > > > > > is
> > > > > > > > > > a nice and simple approach that is backward compatible.
> > > > > > > > > >
> > > > > > > > > > The KIP looks good so far, but I do have two specific
> > > > suggestions
> > > > > > to
> > > > > > > > make
> > > > > > > > > > things just a bit more explicit. First, both the "Public
> > API"
> > > > and
> > > > > > > > > "Proposed
> > > > > > > > > > Changes" sections could be more explicit that the methods
> > in
> > > > the
> > > > > > > > proposal
> > > > > > > > > > are being added; as it's currently written a reader must
> > > infer
> > > > > > that.
> > > > > > > > > > Second, the "Proposed Changes" section needs to more
> > clearly
> > > > > > specify
> > > > > > > > that
> > > > > > > > > > the WorkerSourceTask will now use the new fromConnectData
> > > > method
> > > > > > with
> > > > > > > > the
> > > > > > > > > > headers instead of the existing method, and that the
> > > > WorkerSinkTask
> > > > > > > > will
> > > > > > > > > > now use the toConnectData method with the headers instead
> > of
> > > > the
> > > > > > > > existing
> > > > > > > > > > method.
> > > > > > > > > >
> > > > > > > > > > Best regards,
> > > > > > > > > >
> > > > > > > > > > Randall
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > On Mon, Mar 11, 2019 at 11:01 PM Yaroslav Tkachenko <
> > > > > > > > sapiensy@gmail.com>
> > > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > >> Hello,
> > > > > > > > > >>
> > > > > > > > > >> I'd like to propose a KIP that extends Kafka Connect
> > > Converter
> > > > > > > > > interface:
> > > > > > > > > >>
> > > > > > > > > >>
> > > > > > > > >
> > > > > > > >
> > > > > >
> > > >
> > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-440%3A+Extend+Connect+Converter+to+support+headers
> > > > > > > > > >>
> > > > > > > > > >> Thanks for considering!
> > > > > > > > > >>
> > > > > > > > > >> --
> > > > > > > > > >> Yaroslav Tkachenko
> > > > > > > > > >> sap1ens.com
> > > > > > > > > >>
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-440: Extend Connect Converter to support headers
Posted by Randall Hauch <rh...@gmail.com>.
Have we started a vote on this? I don't see the separate "[VOTE]" thread.
On Mon, Apr 29, 2019 at 6:19 PM Konstantine Karantasis <
konstantine@confluent.io> wrote:
> Thanks Yaroslav, this KIP LGTM now too!
>
> To give some context regarding my previous comment: headers in Connect
> would probably have followed a similar approach if default methods in
> interfaces could be used at the time. But we could not have assumed java 8
> or later yet in the AK version that Connect headers were added, so, I
> believe, that led to two different converter interfaces.
>
> Thanks for the nicely written KIP!
> Konstantine
>
>
>
> On Mon, Apr 29, 2019 at 3:39 PM Randall Hauch <rh...@gmail.com> wrote:
>
> > Thanks for the update. Yes, IMO this KIP is ready for a vote.
> >
> > On Fri, Apr 26, 2019 at 12:15 AM sapiensy@gmail.com <sa...@gmail.com>
> > wrote:
> >
> > > Hi Randall, Konstantine,
> > >
> > > I've updated the KIP to reflect the details we discussed here. Let me
> > know
> > > if it looks good and we can go to the voting phase.
> > >
> > > Thanks!
> > >
> > > On 2019/04/22 21:07:31, Randall Hauch <rh...@gmail.com> wrote:
> > > > I think it would be helpful to clarify this in the KIP, just so that
> > > > readers are aware that the headers will be the raw header bytes that
> > are
> > > > the same as what is in the Kafka record.
> > > >
> > > > The alternative I was referring to is exposing the Connect `Headers`
> > > > interface, which is different.
> > > >
> > > > On Mon, Apr 22, 2019 at 1:45 PM sapiensy@gmail.com <
> sapiensy@gmail.com
> > >
> > > > wrote:
> > > >
> > > > > Hi Konstantine, Randall,
> > > > >
> > > > > As you can see in the updated Converter interface, it always
> operates
> > > on
> > > > > `org.apache.kafka.common.header.Headers`.
> > > > >
> > > > > WorkerSinkTask simply uses Kafka message headers and passes them to
> > the
> > > > > `toConnectData` method.
> > > > >
> > > > > WorkerSourceTask leverages header converter to extract
> RecordHeaders,
> > > > > which implements Headers interface. Then RecordHeaders are passed
> to
> > > the
> > > > > `fromConnectData` method.
> > > > >
> > > > > So header converter is used as a way to get headers when converting
> > > data
> > > > > from internal Connect format to Kafka messages (cause there is no
> > > other way
> > > > > to get the headers in this case).
> > > > >
> > > > > I can add this to the KIP if it's helpful.
> > > > >
> > > > > Randall, what is the alternative approach you're referring to?
> > > > >
> > > > > On 2019/04/22 18:09:24, Randall Hauch <rh...@gmail.com> wrote:
> > > > > > Konstantine raises a good point. Which `Headers` is being
> > referenced
> > > in
> > > > > the
> > > > > > API? The Connect `org.apache.kafka.connect.header.Headers` would
> > > > > correspond
> > > > > > to what was already deserialized by the `HeaderConverter` or what
> > > will
> > > > > yet
> > > > > > be serialized by the `HeaderConverter`. Alternatively, the
> common `
> > > > > > org.apache.kafka.common.header.Headers` would correspond to the
> raw
> > > > > header
> > > > > > pairs from the underlying Kafka record.
> > > > > >
> > > > > > So, we probably want to be a bit more specific, and also mention
> > > why. And
> > > > > > we probably want to mention the other approach in the rejected
> > > > > alternatives.
> > > > > >
> > > > > > Best regards,
> > > > > >
> > > > > > Randall
> > > > > >
> > > > > > On Mon, Apr 22, 2019 at 11:59 AM Konstantine Karantasis <
> > > > > > konstantine@confluent.io> wrote:
> > > > > >
> > > > > > > Thanks for the KIP Yaroslav!
> > > > > > >
> > > > > > > Apologies for the late comment. However, after reading the KIP
> > it's
> > > > > still
> > > > > > > not very clear to me what happens to the existing
> > > > > > > HeaderConverter interface and what's the expectation for
> existing
> > > code
> > > > > > > implementing this interface out there.
> > > > > > >
> > > > > > > Looking at the PR I see that the existing code is leaving the
> > > existing
> > > > > > > connect headers conversion unaffected. I'd expect by reading
> the
> > > KIP to
> > > > > > > understand what's the interplay of the current proposal with
> the
> > > > > existing
> > > > > > > implementation of KIP-145 that introduced headers in Connect.
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Konstantine
> > > > > > >
> > > > > > > On Mon, Apr 22, 2019 at 9:07 AM Randall Hauch <
> rhauch@gmail.com>
> > > > > wrote:
> > > > > > >
> > > > > > > > Thanks for updating the KIP. It looks good to me, and since
> > there
> > > > > haven't
> > > > > > > > been any other issue mentioned in this month-long thread,
> it's
> > > > > probably
> > > > > > > > fine to start a vote.
> > > > > > > >
> > > > > > > > Best regards,
> > > > > > > >
> > > > > > > > Randall
> > > > > > > >
> > > > > > > > On Tue, Apr 2, 2019 at 3:12 PM Randall Hauch <
> rhauch@gmail.com
> > >
> > > > > wrote:
> > > > > > > >
> > > > > > > > > Thanks for the submission, Yaroslav -- and for building on
> > the
> > > > > > > suggestion
> > > > > > > > > of Jeremy C in
> > > https://issues.apache.org/jira/browse/KAFKA-7273.
> > > > > This
> > > > > > > is
> > > > > > > > > a nice and simple approach that is backward compatible.
> > > > > > > > >
> > > > > > > > > The KIP looks good so far, but I do have two specific
> > > suggestions
> > > > > to
> > > > > > > make
> > > > > > > > > things just a bit more explicit. First, both the "Public
> API"
> > > and
> > > > > > > > "Proposed
> > > > > > > > > Changes" sections could be more explicit that the methods
> in
> > > the
> > > > > > > proposal
> > > > > > > > > are being added; as it's currently written a reader must
> > infer
> > > > > that.
> > > > > > > > > Second, the "Proposed Changes" section needs to more
> clearly
> > > > > specify
> > > > > > > that
> > > > > > > > > the WorkerSourceTask will now use the new fromConnectData
> > > method
> > > > > with
> > > > > > > the
> > > > > > > > > headers instead of the existing method, and that the
> > > WorkerSinkTask
> > > > > > > will
> > > > > > > > > now use the toConnectData method with the headers instead
> of
> > > the
> > > > > > > existing
> > > > > > > > > method.
> > > > > > > > >
> > > > > > > > > Best regards,
> > > > > > > > >
> > > > > > > > > Randall
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > On Mon, Mar 11, 2019 at 11:01 PM Yaroslav Tkachenko <
> > > > > > > sapiensy@gmail.com>
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > >> Hello,
> > > > > > > > >>
> > > > > > > > >> I'd like to propose a KIP that extends Kafka Connect
> > Converter
> > > > > > > > interface:
> > > > > > > > >>
> > > > > > > > >>
> > > > > > > >
> > > > > > >
> > > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-440%3A+Extend+Connect+Converter+to+support+headers
> > > > > > > > >>
> > > > > > > > >> Thanks for considering!
> > > > > > > > >>
> > > > > > > > >> --
> > > > > > > > >> Yaroslav Tkachenko
> > > > > > > > >> sap1ens.com
> > > > > > > > >>
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-440: Extend Connect Converter to support headers
Posted by Konstantine Karantasis <ko...@confluent.io>.
Thanks Yaroslav, this KIP LGTM now too!
To give some context regarding my previous comment: headers in Connect
would probably have followed a similar approach if default methods in
interfaces could be used at the time. But we could not have assumed java 8
or later yet in the AK version that Connect headers were added, so, I
believe, that led to two different converter interfaces.
Thanks for the nicely written KIP!
Konstantine
On Mon, Apr 29, 2019 at 3:39 PM Randall Hauch <rh...@gmail.com> wrote:
> Thanks for the update. Yes, IMO this KIP is ready for a vote.
>
> On Fri, Apr 26, 2019 at 12:15 AM sapiensy@gmail.com <sa...@gmail.com>
> wrote:
>
> > Hi Randall, Konstantine,
> >
> > I've updated the KIP to reflect the details we discussed here. Let me
> know
> > if it looks good and we can go to the voting phase.
> >
> > Thanks!
> >
> > On 2019/04/22 21:07:31, Randall Hauch <rh...@gmail.com> wrote:
> > > I think it would be helpful to clarify this in the KIP, just so that
> > > readers are aware that the headers will be the raw header bytes that
> are
> > > the same as what is in the Kafka record.
> > >
> > > The alternative I was referring to is exposing the Connect `Headers`
> > > interface, which is different.
> > >
> > > On Mon, Apr 22, 2019 at 1:45 PM sapiensy@gmail.com <sapiensy@gmail.com
> >
> > > wrote:
> > >
> > > > Hi Konstantine, Randall,
> > > >
> > > > As you can see in the updated Converter interface, it always operates
> > on
> > > > `org.apache.kafka.common.header.Headers`.
> > > >
> > > > WorkerSinkTask simply uses Kafka message headers and passes them to
> the
> > > > `toConnectData` method.
> > > >
> > > > WorkerSourceTask leverages header converter to extract RecordHeaders,
> > > > which implements Headers interface. Then RecordHeaders are passed to
> > the
> > > > `fromConnectData` method.
> > > >
> > > > So header converter is used as a way to get headers when converting
> > data
> > > > from internal Connect format to Kafka messages (cause there is no
> > other way
> > > > to get the headers in this case).
> > > >
> > > > I can add this to the KIP if it's helpful.
> > > >
> > > > Randall, what is the alternative approach you're referring to?
> > > >
> > > > On 2019/04/22 18:09:24, Randall Hauch <rh...@gmail.com> wrote:
> > > > > Konstantine raises a good point. Which `Headers` is being
> referenced
> > in
> > > > the
> > > > > API? The Connect `org.apache.kafka.connect.header.Headers` would
> > > > correspond
> > > > > to what was already deserialized by the `HeaderConverter` or what
> > will
> > > > yet
> > > > > be serialized by the `HeaderConverter`. Alternatively, the common `
> > > > > org.apache.kafka.common.header.Headers` would correspond to the raw
> > > > header
> > > > > pairs from the underlying Kafka record.
> > > > >
> > > > > So, we probably want to be a bit more specific, and also mention
> > why. And
> > > > > we probably want to mention the other approach in the rejected
> > > > alternatives.
> > > > >
> > > > > Best regards,
> > > > >
> > > > > Randall
> > > > >
> > > > > On Mon, Apr 22, 2019 at 11:59 AM Konstantine Karantasis <
> > > > > konstantine@confluent.io> wrote:
> > > > >
> > > > > > Thanks for the KIP Yaroslav!
> > > > > >
> > > > > > Apologies for the late comment. However, after reading the KIP
> it's
> > > > still
> > > > > > not very clear to me what happens to the existing
> > > > > > HeaderConverter interface and what's the expectation for existing
> > code
> > > > > > implementing this interface out there.
> > > > > >
> > > > > > Looking at the PR I see that the existing code is leaving the
> > existing
> > > > > > connect headers conversion unaffected. I'd expect by reading the
> > KIP to
> > > > > > understand what's the interplay of the current proposal with the
> > > > existing
> > > > > > implementation of KIP-145 that introduced headers in Connect.
> > > > > >
> > > > > > Thanks,
> > > > > > Konstantine
> > > > > >
> > > > > > On Mon, Apr 22, 2019 at 9:07 AM Randall Hauch <rh...@gmail.com>
> > > > wrote:
> > > > > >
> > > > > > > Thanks for updating the KIP. It looks good to me, and since
> there
> > > > haven't
> > > > > > > been any other issue mentioned in this month-long thread, it's
> > > > probably
> > > > > > > fine to start a vote.
> > > > > > >
> > > > > > > Best regards,
> > > > > > >
> > > > > > > Randall
> > > > > > >
> > > > > > > On Tue, Apr 2, 2019 at 3:12 PM Randall Hauch <rhauch@gmail.com
> >
> > > > wrote:
> > > > > > >
> > > > > > > > Thanks for the submission, Yaroslav -- and for building on
> the
> > > > > > suggestion
> > > > > > > > of Jeremy C in
> > https://issues.apache.org/jira/browse/KAFKA-7273.
> > > > This
> > > > > > is
> > > > > > > > a nice and simple approach that is backward compatible.
> > > > > > > >
> > > > > > > > The KIP looks good so far, but I do have two specific
> > suggestions
> > > > to
> > > > > > make
> > > > > > > > things just a bit more explicit. First, both the "Public API"
> > and
> > > > > > > "Proposed
> > > > > > > > Changes" sections could be more explicit that the methods in
> > the
> > > > > > proposal
> > > > > > > > are being added; as it's currently written a reader must
> infer
> > > > that.
> > > > > > > > Second, the "Proposed Changes" section needs to more clearly
> > > > specify
> > > > > > that
> > > > > > > > the WorkerSourceTask will now use the new fromConnectData
> > method
> > > > with
> > > > > > the
> > > > > > > > headers instead of the existing method, and that the
> > WorkerSinkTask
> > > > > > will
> > > > > > > > now use the toConnectData method with the headers instead of
> > the
> > > > > > existing
> > > > > > > > method.
> > > > > > > >
> > > > > > > > Best regards,
> > > > > > > >
> > > > > > > > Randall
> > > > > > > >
> > > > > > > >
> > > > > > > > On Mon, Mar 11, 2019 at 11:01 PM Yaroslav Tkachenko <
> > > > > > sapiensy@gmail.com>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > >> Hello,
> > > > > > > >>
> > > > > > > >> I'd like to propose a KIP that extends Kafka Connect
> Converter
> > > > > > > interface:
> > > > > > > >>
> > > > > > > >>
> > > > > > >
> > > > > >
> > > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-440%3A+Extend+Connect+Converter+to+support+headers
> > > > > > > >>
> > > > > > > >> Thanks for considering!
> > > > > > > >>
> > > > > > > >> --
> > > > > > > >> Yaroslav Tkachenko
> > > > > > > >> sap1ens.com
> > > > > > > >>
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-440: Extend Connect Converter to support headers
Posted by Randall Hauch <rh...@gmail.com>.
Thanks for the update. Yes, IMO this KIP is ready for a vote.
On Fri, Apr 26, 2019 at 12:15 AM sapiensy@gmail.com <sa...@gmail.com>
wrote:
> Hi Randall, Konstantine,
>
> I've updated the KIP to reflect the details we discussed here. Let me know
> if it looks good and we can go to the voting phase.
>
> Thanks!
>
> On 2019/04/22 21:07:31, Randall Hauch <rh...@gmail.com> wrote:
> > I think it would be helpful to clarify this in the KIP, just so that
> > readers are aware that the headers will be the raw header bytes that are
> > the same as what is in the Kafka record.
> >
> > The alternative I was referring to is exposing the Connect `Headers`
> > interface, which is different.
> >
> > On Mon, Apr 22, 2019 at 1:45 PM sapiensy@gmail.com <sa...@gmail.com>
> > wrote:
> >
> > > Hi Konstantine, Randall,
> > >
> > > As you can see in the updated Converter interface, it always operates
> on
> > > `org.apache.kafka.common.header.Headers`.
> > >
> > > WorkerSinkTask simply uses Kafka message headers and passes them to the
> > > `toConnectData` method.
> > >
> > > WorkerSourceTask leverages header converter to extract RecordHeaders,
> > > which implements Headers interface. Then RecordHeaders are passed to
> the
> > > `fromConnectData` method.
> > >
> > > So header converter is used as a way to get headers when converting
> data
> > > from internal Connect format to Kafka messages (cause there is no
> other way
> > > to get the headers in this case).
> > >
> > > I can add this to the KIP if it's helpful.
> > >
> > > Randall, what is the alternative approach you're referring to?
> > >
> > > On 2019/04/22 18:09:24, Randall Hauch <rh...@gmail.com> wrote:
> > > > Konstantine raises a good point. Which `Headers` is being referenced
> in
> > > the
> > > > API? The Connect `org.apache.kafka.connect.header.Headers` would
> > > correspond
> > > > to what was already deserialized by the `HeaderConverter` or what
> will
> > > yet
> > > > be serialized by the `HeaderConverter`. Alternatively, the common `
> > > > org.apache.kafka.common.header.Headers` would correspond to the raw
> > > header
> > > > pairs from the underlying Kafka record.
> > > >
> > > > So, we probably want to be a bit more specific, and also mention
> why. And
> > > > we probably want to mention the other approach in the rejected
> > > alternatives.
> > > >
> > > > Best regards,
> > > >
> > > > Randall
> > > >
> > > > On Mon, Apr 22, 2019 at 11:59 AM Konstantine Karantasis <
> > > > konstantine@confluent.io> wrote:
> > > >
> > > > > Thanks for the KIP Yaroslav!
> > > > >
> > > > > Apologies for the late comment. However, after reading the KIP it's
> > > still
> > > > > not very clear to me what happens to the existing
> > > > > HeaderConverter interface and what's the expectation for existing
> code
> > > > > implementing this interface out there.
> > > > >
> > > > > Looking at the PR I see that the existing code is leaving the
> existing
> > > > > connect headers conversion unaffected. I'd expect by reading the
> KIP to
> > > > > understand what's the interplay of the current proposal with the
> > > existing
> > > > > implementation of KIP-145 that introduced headers in Connect.
> > > > >
> > > > > Thanks,
> > > > > Konstantine
> > > > >
> > > > > On Mon, Apr 22, 2019 at 9:07 AM Randall Hauch <rh...@gmail.com>
> > > wrote:
> > > > >
> > > > > > Thanks for updating the KIP. It looks good to me, and since there
> > > haven't
> > > > > > been any other issue mentioned in this month-long thread, it's
> > > probably
> > > > > > fine to start a vote.
> > > > > >
> > > > > > Best regards,
> > > > > >
> > > > > > Randall
> > > > > >
> > > > > > On Tue, Apr 2, 2019 at 3:12 PM Randall Hauch <rh...@gmail.com>
> > > wrote:
> > > > > >
> > > > > > > Thanks for the submission, Yaroslav -- and for building on the
> > > > > suggestion
> > > > > > > of Jeremy C in
> https://issues.apache.org/jira/browse/KAFKA-7273.
> > > This
> > > > > is
> > > > > > > a nice and simple approach that is backward compatible.
> > > > > > >
> > > > > > > The KIP looks good so far, but I do have two specific
> suggestions
> > > to
> > > > > make
> > > > > > > things just a bit more explicit. First, both the "Public API"
> and
> > > > > > "Proposed
> > > > > > > Changes" sections could be more explicit that the methods in
> the
> > > > > proposal
> > > > > > > are being added; as it's currently written a reader must infer
> > > that.
> > > > > > > Second, the "Proposed Changes" section needs to more clearly
> > > specify
> > > > > that
> > > > > > > the WorkerSourceTask will now use the new fromConnectData
> method
> > > with
> > > > > the
> > > > > > > headers instead of the existing method, and that the
> WorkerSinkTask
> > > > > will
> > > > > > > now use the toConnectData method with the headers instead of
> the
> > > > > existing
> > > > > > > method.
> > > > > > >
> > > > > > > Best regards,
> > > > > > >
> > > > > > > Randall
> > > > > > >
> > > > > > >
> > > > > > > On Mon, Mar 11, 2019 at 11:01 PM Yaroslav Tkachenko <
> > > > > sapiensy@gmail.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > >> Hello,
> > > > > > >>
> > > > > > >> I'd like to propose a KIP that extends Kafka Connect Converter
> > > > > > interface:
> > > > > > >>
> > > > > > >>
> > > > > >
> > > > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-440%3A+Extend+Connect+Converter+to+support+headers
> > > > > > >>
> > > > > > >> Thanks for considering!
> > > > > > >>
> > > > > > >> --
> > > > > > >> Yaroslav Tkachenko
> > > > > > >> sap1ens.com
> > > > > > >>
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-440: Extend Connect Converter to support headers
Posted by sa...@gmail.com,
sa...@gmail.com.
Hi Randall, Konstantine,
I've updated the KIP to reflect the details we discussed here. Let me know if it looks good and we can go to the voting phase.
Thanks!
On 2019/04/22 21:07:31, Randall Hauch <rh...@gmail.com> wrote:
> I think it would be helpful to clarify this in the KIP, just so that
> readers are aware that the headers will be the raw header bytes that are
> the same as what is in the Kafka record.
>
> The alternative I was referring to is exposing the Connect `Headers`
> interface, which is different.
>
> On Mon, Apr 22, 2019 at 1:45 PM sapiensy@gmail.com <sa...@gmail.com>
> wrote:
>
> > Hi Konstantine, Randall,
> >
> > As you can see in the updated Converter interface, it always operates on
> > `org.apache.kafka.common.header.Headers`.
> >
> > WorkerSinkTask simply uses Kafka message headers and passes them to the
> > `toConnectData` method.
> >
> > WorkerSourceTask leverages header converter to extract RecordHeaders,
> > which implements Headers interface. Then RecordHeaders are passed to the
> > `fromConnectData` method.
> >
> > So header converter is used as a way to get headers when converting data
> > from internal Connect format to Kafka messages (cause there is no other way
> > to get the headers in this case).
> >
> > I can add this to the KIP if it's helpful.
> >
> > Randall, what is the alternative approach you're referring to?
> >
> > On 2019/04/22 18:09:24, Randall Hauch <rh...@gmail.com> wrote:
> > > Konstantine raises a good point. Which `Headers` is being referenced in
> > the
> > > API? The Connect `org.apache.kafka.connect.header.Headers` would
> > correspond
> > > to what was already deserialized by the `HeaderConverter` or what will
> > yet
> > > be serialized by the `HeaderConverter`. Alternatively, the common `
> > > org.apache.kafka.common.header.Headers` would correspond to the raw
> > header
> > > pairs from the underlying Kafka record.
> > >
> > > So, we probably want to be a bit more specific, and also mention why. And
> > > we probably want to mention the other approach in the rejected
> > alternatives.
> > >
> > > Best regards,
> > >
> > > Randall
> > >
> > > On Mon, Apr 22, 2019 at 11:59 AM Konstantine Karantasis <
> > > konstantine@confluent.io> wrote:
> > >
> > > > Thanks for the KIP Yaroslav!
> > > >
> > > > Apologies for the late comment. However, after reading the KIP it's
> > still
> > > > not very clear to me what happens to the existing
> > > > HeaderConverter interface and what's the expectation for existing code
> > > > implementing this interface out there.
> > > >
> > > > Looking at the PR I see that the existing code is leaving the existing
> > > > connect headers conversion unaffected. I'd expect by reading the KIP to
> > > > understand what's the interplay of the current proposal with the
> > existing
> > > > implementation of KIP-145 that introduced headers in Connect.
> > > >
> > > > Thanks,
> > > > Konstantine
> > > >
> > > > On Mon, Apr 22, 2019 at 9:07 AM Randall Hauch <rh...@gmail.com>
> > wrote:
> > > >
> > > > > Thanks for updating the KIP. It looks good to me, and since there
> > haven't
> > > > > been any other issue mentioned in this month-long thread, it's
> > probably
> > > > > fine to start a vote.
> > > > >
> > > > > Best regards,
> > > > >
> > > > > Randall
> > > > >
> > > > > On Tue, Apr 2, 2019 at 3:12 PM Randall Hauch <rh...@gmail.com>
> > wrote:
> > > > >
> > > > > > Thanks for the submission, Yaroslav -- and for building on the
> > > > suggestion
> > > > > > of Jeremy C in https://issues.apache.org/jira/browse/KAFKA-7273.
> > This
> > > > is
> > > > > > a nice and simple approach that is backward compatible.
> > > > > >
> > > > > > The KIP looks good so far, but I do have two specific suggestions
> > to
> > > > make
> > > > > > things just a bit more explicit. First, both the "Public API" and
> > > > > "Proposed
> > > > > > Changes" sections could be more explicit that the methods in the
> > > > proposal
> > > > > > are being added; as it's currently written a reader must infer
> > that.
> > > > > > Second, the "Proposed Changes" section needs to more clearly
> > specify
> > > > that
> > > > > > the WorkerSourceTask will now use the new fromConnectData method
> > with
> > > > the
> > > > > > headers instead of the existing method, and that the WorkerSinkTask
> > > > will
> > > > > > now use the toConnectData method with the headers instead of the
> > > > existing
> > > > > > method.
> > > > > >
> > > > > > Best regards,
> > > > > >
> > > > > > Randall
> > > > > >
> > > > > >
> > > > > > On Mon, Mar 11, 2019 at 11:01 PM Yaroslav Tkachenko <
> > > > sapiensy@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > >> Hello,
> > > > > >>
> > > > > >> I'd like to propose a KIP that extends Kafka Connect Converter
> > > > > interface:
> > > > > >>
> > > > > >>
> > > > >
> > > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-440%3A+Extend+Connect+Converter+to+support+headers
> > > > > >>
> > > > > >> Thanks for considering!
> > > > > >>
> > > > > >> --
> > > > > >> Yaroslav Tkachenko
> > > > > >> sap1ens.com
> > > > > >>
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-440: Extend Connect Converter to support headers
Posted by Randall Hauch <rh...@gmail.com>.
I think it would be helpful to clarify this in the KIP, just so that
readers are aware that the headers will be the raw header bytes that are
the same as what is in the Kafka record.
The alternative I was referring to is exposing the Connect `Headers`
interface, which is different.
On Mon, Apr 22, 2019 at 1:45 PM sapiensy@gmail.com <sa...@gmail.com>
wrote:
> Hi Konstantine, Randall,
>
> As you can see in the updated Converter interface, it always operates on
> `org.apache.kafka.common.header.Headers`.
>
> WorkerSinkTask simply uses Kafka message headers and passes them to the
> `toConnectData` method.
>
> WorkerSourceTask leverages header converter to extract RecordHeaders,
> which implements Headers interface. Then RecordHeaders are passed to the
> `fromConnectData` method.
>
> So header converter is used as a way to get headers when converting data
> from internal Connect format to Kafka messages (cause there is no other way
> to get the headers in this case).
>
> I can add this to the KIP if it's helpful.
>
> Randall, what is the alternative approach you're referring to?
>
> On 2019/04/22 18:09:24, Randall Hauch <rh...@gmail.com> wrote:
> > Konstantine raises a good point. Which `Headers` is being referenced in
> the
> > API? The Connect `org.apache.kafka.connect.header.Headers` would
> correspond
> > to what was already deserialized by the `HeaderConverter` or what will
> yet
> > be serialized by the `HeaderConverter`. Alternatively, the common `
> > org.apache.kafka.common.header.Headers` would correspond to the raw
> header
> > pairs from the underlying Kafka record.
> >
> > So, we probably want to be a bit more specific, and also mention why. And
> > we probably want to mention the other approach in the rejected
> alternatives.
> >
> > Best regards,
> >
> > Randall
> >
> > On Mon, Apr 22, 2019 at 11:59 AM Konstantine Karantasis <
> > konstantine@confluent.io> wrote:
> >
> > > Thanks for the KIP Yaroslav!
> > >
> > > Apologies for the late comment. However, after reading the KIP it's
> still
> > > not very clear to me what happens to the existing
> > > HeaderConverter interface and what's the expectation for existing code
> > > implementing this interface out there.
> > >
> > > Looking at the PR I see that the existing code is leaving the existing
> > > connect headers conversion unaffected. I'd expect by reading the KIP to
> > > understand what's the interplay of the current proposal with the
> existing
> > > implementation of KIP-145 that introduced headers in Connect.
> > >
> > > Thanks,
> > > Konstantine
> > >
> > > On Mon, Apr 22, 2019 at 9:07 AM Randall Hauch <rh...@gmail.com>
> wrote:
> > >
> > > > Thanks for updating the KIP. It looks good to me, and since there
> haven't
> > > > been any other issue mentioned in this month-long thread, it's
> probably
> > > > fine to start a vote.
> > > >
> > > > Best regards,
> > > >
> > > > Randall
> > > >
> > > > On Tue, Apr 2, 2019 at 3:12 PM Randall Hauch <rh...@gmail.com>
> wrote:
> > > >
> > > > > Thanks for the submission, Yaroslav -- and for building on the
> > > suggestion
> > > > > of Jeremy C in https://issues.apache.org/jira/browse/KAFKA-7273.
> This
> > > is
> > > > > a nice and simple approach that is backward compatible.
> > > > >
> > > > > The KIP looks good so far, but I do have two specific suggestions
> to
> > > make
> > > > > things just a bit more explicit. First, both the "Public API" and
> > > > "Proposed
> > > > > Changes" sections could be more explicit that the methods in the
> > > proposal
> > > > > are being added; as it's currently written a reader must infer
> that.
> > > > > Second, the "Proposed Changes" section needs to more clearly
> specify
> > > that
> > > > > the WorkerSourceTask will now use the new fromConnectData method
> with
> > > the
> > > > > headers instead of the existing method, and that the WorkerSinkTask
> > > will
> > > > > now use the toConnectData method with the headers instead of the
> > > existing
> > > > > method.
> > > > >
> > > > > Best regards,
> > > > >
> > > > > Randall
> > > > >
> > > > >
> > > > > On Mon, Mar 11, 2019 at 11:01 PM Yaroslav Tkachenko <
> > > sapiensy@gmail.com>
> > > > > wrote:
> > > > >
> > > > >> Hello,
> > > > >>
> > > > >> I'd like to propose a KIP that extends Kafka Connect Converter
> > > > interface:
> > > > >>
> > > > >>
> > > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-440%3A+Extend+Connect+Converter+to+support+headers
> > > > >>
> > > > >> Thanks for considering!
> > > > >>
> > > > >> --
> > > > >> Yaroslav Tkachenko
> > > > >> sap1ens.com
> > > > >>
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-440: Extend Connect Converter to support headers
Posted by sa...@gmail.com,
sa...@gmail.com.
Hi Konstantine, Randall,
As you can see in the updated Converter interface, it always operates on `org.apache.kafka.common.header.Headers`.
WorkerSinkTask simply uses Kafka message headers and passes them to the `toConnectData` method.
WorkerSourceTask leverages header converter to extract RecordHeaders, which implements Headers interface. Then RecordHeaders are passed to the `fromConnectData` method.
So header converter is used as a way to get headers when converting data from internal Connect format to Kafka messages (cause there is no other way to get the headers in this case).
I can add this to the KIP if it's helpful.
Randall, what is the alternative approach you're referring to?
On 2019/04/22 18:09:24, Randall Hauch <rh...@gmail.com> wrote:
> Konstantine raises a good point. Which `Headers` is being referenced in the
> API? The Connect `org.apache.kafka.connect.header.Headers` would correspond
> to what was already deserialized by the `HeaderConverter` or what will yet
> be serialized by the `HeaderConverter`. Alternatively, the common `
> org.apache.kafka.common.header.Headers` would correspond to the raw header
> pairs from the underlying Kafka record.
>
> So, we probably want to be a bit more specific, and also mention why. And
> we probably want to mention the other approach in the rejected alternatives.
>
> Best regards,
>
> Randall
>
> On Mon, Apr 22, 2019 at 11:59 AM Konstantine Karantasis <
> konstantine@confluent.io> wrote:
>
> > Thanks for the KIP Yaroslav!
> >
> > Apologies for the late comment. However, after reading the KIP it's still
> > not very clear to me what happens to the existing
> > HeaderConverter interface and what's the expectation for existing code
> > implementing this interface out there.
> >
> > Looking at the PR I see that the existing code is leaving the existing
> > connect headers conversion unaffected. I'd expect by reading the KIP to
> > understand what's the interplay of the current proposal with the existing
> > implementation of KIP-145 that introduced headers in Connect.
> >
> > Thanks,
> > Konstantine
> >
> > On Mon, Apr 22, 2019 at 9:07 AM Randall Hauch <rh...@gmail.com> wrote:
> >
> > > Thanks for updating the KIP. It looks good to me, and since there haven't
> > > been any other issue mentioned in this month-long thread, it's probably
> > > fine to start a vote.
> > >
> > > Best regards,
> > >
> > > Randall
> > >
> > > On Tue, Apr 2, 2019 at 3:12 PM Randall Hauch <rh...@gmail.com> wrote:
> > >
> > > > Thanks for the submission, Yaroslav -- and for building on the
> > suggestion
> > > > of Jeremy C in https://issues.apache.org/jira/browse/KAFKA-7273. This
> > is
> > > > a nice and simple approach that is backward compatible.
> > > >
> > > > The KIP looks good so far, but I do have two specific suggestions to
> > make
> > > > things just a bit more explicit. First, both the "Public API" and
> > > "Proposed
> > > > Changes" sections could be more explicit that the methods in the
> > proposal
> > > > are being added; as it's currently written a reader must infer that.
> > > > Second, the "Proposed Changes" section needs to more clearly specify
> > that
> > > > the WorkerSourceTask will now use the new fromConnectData method with
> > the
> > > > headers instead of the existing method, and that the WorkerSinkTask
> > will
> > > > now use the toConnectData method with the headers instead of the
> > existing
> > > > method.
> > > >
> > > > Best regards,
> > > >
> > > > Randall
> > > >
> > > >
> > > > On Mon, Mar 11, 2019 at 11:01 PM Yaroslav Tkachenko <
> > sapiensy@gmail.com>
> > > > wrote:
> > > >
> > > >> Hello,
> > > >>
> > > >> I'd like to propose a KIP that extends Kafka Connect Converter
> > > interface:
> > > >>
> > > >>
> > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-440%3A+Extend+Connect+Converter+to+support+headers
> > > >>
> > > >> Thanks for considering!
> > > >>
> > > >> --
> > > >> Yaroslav Tkachenko
> > > >> sap1ens.com
> > > >>
> > > >
> > >
> >
>
Re: [DISCUSS] KIP-440: Extend Connect Converter to support headers
Posted by Randall Hauch <rh...@gmail.com>.
Konstantine raises a good point. Which `Headers` is being referenced in the
API? The Connect `org.apache.kafka.connect.header.Headers` would correspond
to what was already deserialized by the `HeaderConverter` or what will yet
be serialized by the `HeaderConverter`. Alternatively, the common `
org.apache.kafka.common.header.Headers` would correspond to the raw header
pairs from the underlying Kafka record.
So, we probably want to be a bit more specific, and also mention why. And
we probably want to mention the other approach in the rejected alternatives.
Best regards,
Randall
On Mon, Apr 22, 2019 at 11:59 AM Konstantine Karantasis <
konstantine@confluent.io> wrote:
> Thanks for the KIP Yaroslav!
>
> Apologies for the late comment. However, after reading the KIP it's still
> not very clear to me what happens to the existing
> HeaderConverter interface and what's the expectation for existing code
> implementing this interface out there.
>
> Looking at the PR I see that the existing code is leaving the existing
> connect headers conversion unaffected. I'd expect by reading the KIP to
> understand what's the interplay of the current proposal with the existing
> implementation of KIP-145 that introduced headers in Connect.
>
> Thanks,
> Konstantine
>
> On Mon, Apr 22, 2019 at 9:07 AM Randall Hauch <rh...@gmail.com> wrote:
>
> > Thanks for updating the KIP. It looks good to me, and since there haven't
> > been any other issue mentioned in this month-long thread, it's probably
> > fine to start a vote.
> >
> > Best regards,
> >
> > Randall
> >
> > On Tue, Apr 2, 2019 at 3:12 PM Randall Hauch <rh...@gmail.com> wrote:
> >
> > > Thanks for the submission, Yaroslav -- and for building on the
> suggestion
> > > of Jeremy C in https://issues.apache.org/jira/browse/KAFKA-7273. This
> is
> > > a nice and simple approach that is backward compatible.
> > >
> > > The KIP looks good so far, but I do have two specific suggestions to
> make
> > > things just a bit more explicit. First, both the "Public API" and
> > "Proposed
> > > Changes" sections could be more explicit that the methods in the
> proposal
> > > are being added; as it's currently written a reader must infer that.
> > > Second, the "Proposed Changes" section needs to more clearly specify
> that
> > > the WorkerSourceTask will now use the new fromConnectData method with
> the
> > > headers instead of the existing method, and that the WorkerSinkTask
> will
> > > now use the toConnectData method with the headers instead of the
> existing
> > > method.
> > >
> > > Best regards,
> > >
> > > Randall
> > >
> > >
> > > On Mon, Mar 11, 2019 at 11:01 PM Yaroslav Tkachenko <
> sapiensy@gmail.com>
> > > wrote:
> > >
> > >> Hello,
> > >>
> > >> I'd like to propose a KIP that extends Kafka Connect Converter
> > interface:
> > >>
> > >>
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-440%3A+Extend+Connect+Converter+to+support+headers
> > >>
> > >> Thanks for considering!
> > >>
> > >> --
> > >> Yaroslav Tkachenko
> > >> sap1ens.com
> > >>
> > >
> >
>
Re: [DISCUSS] KIP-440: Extend Connect Converter to support headers
Posted by Konstantine Karantasis <ko...@confluent.io>.
Thanks for the KIP Yaroslav!
Apologies for the late comment. However, after reading the KIP it's still
not very clear to me what happens to the existing
HeaderConverter interface and what's the expectation for existing code
implementing this interface out there.
Looking at the PR I see that the existing code is leaving the existing
connect headers conversion unaffected. I'd expect by reading the KIP to
understand what's the interplay of the current proposal with the existing
implementation of KIP-145 that introduced headers in Connect.
Thanks,
Konstantine
On Mon, Apr 22, 2019 at 9:07 AM Randall Hauch <rh...@gmail.com> wrote:
> Thanks for updating the KIP. It looks good to me, and since there haven't
> been any other issue mentioned in this month-long thread, it's probably
> fine to start a vote.
>
> Best regards,
>
> Randall
>
> On Tue, Apr 2, 2019 at 3:12 PM Randall Hauch <rh...@gmail.com> wrote:
>
> > Thanks for the submission, Yaroslav -- and for building on the suggestion
> > of Jeremy C in https://issues.apache.org/jira/browse/KAFKA-7273. This is
> > a nice and simple approach that is backward compatible.
> >
> > The KIP looks good so far, but I do have two specific suggestions to make
> > things just a bit more explicit. First, both the "Public API" and
> "Proposed
> > Changes" sections could be more explicit that the methods in the proposal
> > are being added; as it's currently written a reader must infer that.
> > Second, the "Proposed Changes" section needs to more clearly specify that
> > the WorkerSourceTask will now use the new fromConnectData method with the
> > headers instead of the existing method, and that the WorkerSinkTask will
> > now use the toConnectData method with the headers instead of the existing
> > method.
> >
> > Best regards,
> >
> > Randall
> >
> >
> > On Mon, Mar 11, 2019 at 11:01 PM Yaroslav Tkachenko <sa...@gmail.com>
> > wrote:
> >
> >> Hello,
> >>
> >> I'd like to propose a KIP that extends Kafka Connect Converter
> interface:
> >>
> >>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-440%3A+Extend+Connect+Converter+to+support+headers
> >>
> >> Thanks for considering!
> >>
> >> --
> >> Yaroslav Tkachenko
> >> sap1ens.com
> >>
> >
>
Re: [DISCUSS] KIP-440: Extend Connect Converter to support headers
Posted by Randall Hauch <rh...@gmail.com>.
Thanks for updating the KIP. It looks good to me, and since there haven't
been any other issue mentioned in this month-long thread, it's probably
fine to start a vote.
Best regards,
Randall
On Tue, Apr 2, 2019 at 3:12 PM Randall Hauch <rh...@gmail.com> wrote:
> Thanks for the submission, Yaroslav -- and for building on the suggestion
> of Jeremy C in https://issues.apache.org/jira/browse/KAFKA-7273. This is
> a nice and simple approach that is backward compatible.
>
> The KIP looks good so far, but I do have two specific suggestions to make
> things just a bit more explicit. First, both the "Public API" and "Proposed
> Changes" sections could be more explicit that the methods in the proposal
> are being added; as it's currently written a reader must infer that.
> Second, the "Proposed Changes" section needs to more clearly specify that
> the WorkerSourceTask will now use the new fromConnectData method with the
> headers instead of the existing method, and that the WorkerSinkTask will
> now use the toConnectData method with the headers instead of the existing
> method.
>
> Best regards,
>
> Randall
>
>
> On Mon, Mar 11, 2019 at 11:01 PM Yaroslav Tkachenko <sa...@gmail.com>
> wrote:
>
>> Hello,
>>
>> I'd like to propose a KIP that extends Kafka Connect Converter interface:
>>
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-440%3A+Extend+Connect+Converter+to+support+headers
>>
>> Thanks for considering!
>>
>> --
>> Yaroslav Tkachenko
>> sap1ens.com
>>
>
Re: [DISCUSS] KIP-440: Extend Connect Converter to support headers
Posted by Randall Hauch <rh...@gmail.com>.
Thanks for the submission, Yaroslav -- and for building on the suggestion
of Jeremy C in https://issues.apache.org/jira/browse/KAFKA-7273. This is a
nice and simple approach that is backward compatible.
The KIP looks good so far, but I do have two specific suggestions to make
things just a bit more explicit. First, both the "Public API" and "Proposed
Changes" sections could be more explicit that the methods in the proposal
are being added; as it's currently written a reader must infer that.
Second, the "Proposed Changes" section needs to more clearly specify that
the WorkerSourceTask will now use the new fromConnectData method with the
headers instead of the existing method, and that the WorkerSinkTask will
now use the toConnectData method with the headers instead of the existing
method.
Best regards,
Randall
On Mon, Mar 11, 2019 at 11:01 PM Yaroslav Tkachenko <sa...@gmail.com>
wrote:
> Hello,
>
> I'd like to propose a KIP that extends Kafka Connect Converter interface:
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-440%3A+Extend+Connect+Converter+to+support+headers
>
> Thanks for considering!
>
> --
> Yaroslav Tkachenko
> sap1ens.com
>