Thanks for the votes, everyone. This KIP passes with three binding +1 votes and 4 non-binding +1s (including mine).
Randall On Tue, Jan 23, 2018 at 3:19 PM, Michael Pearce <michael.pea...@ig.com> wrote: > +1 (non-binding) > > I personally think the current proposed Converters described in the KIP > are good, as Randall states it keeps it more in line with the pattern of > the Converter methods, I think this is a good reason. > > > > -----Original Message----- > From: Jason Gustafson [mailto:ja...@confluent.io] > Sent: Tuesday, January 23, 2018 8:03 PM > To: dev@kafka.apache.org > Subject: Re: [VOTE] KIP-145: Expose Record Headers in Kafka Connect > > 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 <rha...@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 <ja...@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 <yuzhih...@gmail.com> wrote: > > > > > > > +1 > > > > > > > > On Mon, Jan 22, 2018 at 2:48 PM, Gwen Shapira <g...@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 < > > > > > konstant...@confluent.io> wrote: > > > > > > > > > > > Great addition! > > > > > > > > > > > > +1 (non-binding) > > > > > > > > > > > > Konstantine > > > > > > > > > > > > On Sun, Jan 21, 2018 at 7:26 PM, Ewen Cheslack-Postava < > > > > > e...@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 > > > > > > > <rha...@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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The information contained in this email is strictly confidential and for > the use of the addressee only, unless otherwise indicated. If you are not > the intended recipient, please do not read, copy, use or disclose to others > this message or any attachment. Please also notify the sender by replying > to this email or by telephone (+44(020 7896 0011) and then delete the email > and any copies of it. Opinions, conclusion (etc) that do not relate to the > official business of this company shall be understood as neither given nor > endorsed by it. IG is a trading name of IG Markets Limited (a company > registered in England and Wales, company number 04008957) and IG Index > Limited (a company registered in England and Wales, company number > 01190902). Registered address at Cannon Bridge House, 25 Dowgate Hill, > London EC4R 2YA. Both IG Markets Limited (register number 195355) and IG > Index Limited (register number 114059) are authorised and regulated by the > Financial Conduct Authority. >
