From dev-return-103657-archive-asf-public=cust-asf.ponee.io@kafka.apache.org Mon Apr 29 23:19:09 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id 4282A18061A for ; Tue, 30 Apr 2019 01:19:09 +0200 (CEST) Received: (qmail 26503 invoked by uid 500); 29 Apr 2019 23:19:06 -0000 Mailing-List: contact dev-help@kafka.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@kafka.apache.org Delivered-To: mailing list dev@kafka.apache.org Received: (qmail 26122 invoked by uid 99); 29 Apr 2019 23:19:05 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd1-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 29 Apr 2019 23:19:05 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd1-us-west.apache.org (ASF Mail Server at spamd1-us-west.apache.org) with ESMTP id AF8A0C1023 for ; Mon, 29 Apr 2019 23:19:04 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd1-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 1.798 X-Spam-Level: * X-Spam-Status: No, score=1.798 tagged_above=-999 required=6.31 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=2, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-0.001, SPF_PASS=-0.001] autolearn=disabled Authentication-Results: spamd1-us-west.apache.org (amavisd-new); dkim=pass (2048-bit key) header.d=confluent.io Received: from mx1-lw-eu.apache.org ([10.40.0.8]) by localhost (spamd1-us-west.apache.org [10.40.0.7]) (amavisd-new, port 10024) with ESMTP id v7U4ck-akn5C for ; Mon, 29 Apr 2019 23:19:02 +0000 (UTC) Received: from mail-lj1-f195.google.com (mail-lj1-f195.google.com [209.85.208.195]) by mx1-lw-eu.apache.org (ASF Mail Server at mx1-lw-eu.apache.org) with ESMTPS id DA80061194 for ; Mon, 29 Apr 2019 23:19:01 +0000 (UTC) Received: by mail-lj1-f195.google.com with SMTP id m18so6358544lje.12 for ; Mon, 29 Apr 2019 16:19:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=confluent.io; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=KeFP9CLHrnVpbxDKQ/hucCppuAnUylRbJ6nVauUa6tU=; b=Y5+gs5TUXHhlvf08z5THUq+nHI+2fbT9Vr8UrbbiXOqwQjWwW6IJwOOhjhLpWxL7h5 sevgVI2Ij9Jmp7HIuudtAfwbxp60PN5QN91iNlDCvoszFikcXyjdF/9hXEwn74gAn2eA 1rYiFbeu/M8n7CYkA0IJa3LBZHd5YCOZT9ZwZtH0nJ2VuwThd5bLxiRkwslSivL5jucg Jsi4giVRkXn+wJ6UeTHMUSGSULlDb+obPV47G2UAetd2qE1ie1HRgt/CPwZxV7GPv+Pz FNEaO2MjWhVI1t8iM/Ysdsw7Aq8r13osocwAUQ7yZXazkuu8VhXuagAdWWKO5kIZtc8E /OWQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=KeFP9CLHrnVpbxDKQ/hucCppuAnUylRbJ6nVauUa6tU=; b=bCyO+oTTFY7qy796XCZ2zsqh4Zv1v8NRBlJ+UsIozDfytEsE9Cyvm8jkuuB/YY+Zyo z04OgOuYWTYrZsndVJz/zeqkuRXL91+7ddutM1YbnY0d3HNZZhdUi3RwxJ5YSIOr8Es9 6yTw2Az20DCmdIELbZCve23/+IYf1pNuC0ElHRagmXeRhbCEE5Ms2m5oNjVuzPL7pnK0 +sNtX16WJ0X0EDyIu2LzlmQrJTLAxgm4Rzr/WltgkJoeQa1T5BomGHEsJ1ybVYThcr0y XcYA5zqMs7F4zDR+ltU454QdB11dETLA/TgSqJOF5fwraCoHEYX9gx7RCNZjwmp98wwc +vcw== X-Gm-Message-State: APjAAAWvJgs6o0gZYFzFuMNGmUsCxa5l/muaC6HMuqGY3LE9mEyb7fCg yvlen9w38UPI6jRacWUM+nY2yLfYFAL0hJQrQC5zOrnI X-Google-Smtp-Source: APXvYqzMlMBOIbzU+/Y4V1gOtxdTPKM0gV3YIcXXzwjdY8uWzsvUOhuadaYAnHJCaGkKWmImzssq9RZXFMKG4IQWBVQ= X-Received: by 2002:a2e:6a16:: with SMTP id f22mr997303ljc.40.1556579941028; Mon, 29 Apr 2019 16:19:01 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Konstantine Karantasis Date: Mon, 29 Apr 2019 16:18:50 -0700 Message-ID: Subject: Re: [DISCUSS] KIP-440: Extend Connect Converter to support headers To: dev@kafka.apache.org Content-Type: multipart/alternative; boundary="0000000000007331620587b382dd" --0000000000007331620587b382dd Content-Type: text/plain; charset="UTF-8" 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 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 > 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 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 > > > > 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 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 > > > > 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 > > > > > 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 > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --0000000000007331620587b382dd--