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 <rha...@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 < > konstant...@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 <rha...@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 <rha...@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 < > > sapie...@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 > > > >> > > > > > > > > > >
