From dev-return-92956-archive-asf-public=cust-asf.ponee.io@kafka.apache.org Tue Apr 3 23:16:41 2018 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 [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id 4859818064D for ; Tue, 3 Apr 2018 23:16:41 +0200 (CEST) Received: (qmail 54501 invoked by uid 500); 3 Apr 2018 21:16:39 -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 54456 invoked by uid 99); 3 Apr 2018 21:16:39 -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; Tue, 03 Apr 2018 21:16:39 +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 BBA72C0B77 for ; Tue, 3 Apr 2018 21:16:38 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd1-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 1.979 X-Spam-Level: * X-Spam-Status: No, score=1.979 tagged_above=-999 required=6.31 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, HTML_MESSAGE=2, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, SPF_PASS=-0.001] autolearn=disabled Authentication-Results: spamd1-us-west.apache.org (amavisd-new); dkim=pass (2048-bit key) header.d=confluent-io.20150623.gappssmtp.com Received: from mx1-lw-us.apache.org ([10.40.0.8]) by localhost (spamd1-us-west.apache.org [10.40.0.7]) (amavisd-new, port 10024) with ESMTP id WaGmmkIyvXoQ for ; Tue, 3 Apr 2018 21:16:37 +0000 (UTC) Received: from mail-wm0-f50.google.com (mail-wm0-f50.google.com [74.125.82.50]) by mx1-lw-us.apache.org (ASF Mail Server at mx1-lw-us.apache.org) with ESMTPS id 959B55F1B4 for ; Tue, 3 Apr 2018 21:16:36 +0000 (UTC) Received: by mail-wm0-f50.google.com with SMTP id p9so36188876wmc.3 for ; Tue, 03 Apr 2018 14:16:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=confluent-io.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to; bh=bl6qIUfZNoHgH3FYy5LOXZY2pNC4GoSWPnWMhqJVrRw=; b=JYwLrHpbcuT7UgY58B9OdJi8Tt3UmYKld8kzvEECWjDyJvGlTGMAcF+C1ut08h7yHc kXMldITM/fcPF3/nzMv3OTZCMEhHYlVDCyEr2Y/ebuF64KRCbMYkZHeMPaknboinHQhF ULbs7uHnzNFt84B0SaZdj8mTTCRPZmfd7xUZ58sy5DcTWXBB+9vR2PL+7x+PtDWXWKGv ThuE94qR8rPxcX3LZ4dlfzSmaG5ZPo2f7k/PWAuiFp2Zs4lmgtbaCPdPIy8GjzLuKXJO jzIrUfEv42CfJXIIib2tvZT8jYUSedW+cA0I7KI2iwv1TEXU5HWC+NJU+FkXSUTQIXXW J7jQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to; bh=bl6qIUfZNoHgH3FYy5LOXZY2pNC4GoSWPnWMhqJVrRw=; b=FpZUrzu5uSK+njpDYZZP3CEMdUqQsogm0ZbwLYsEhyojeBJjsyE0Bj9CVHFiHr4Pyu yDVO2fuEF9awmg6JwlkATetS61hMgvv6EaUJBZC6S1pkTmUkswKuf5K44dGsHENDuDau 28FjGqvlkzjqIje42I8ypyV0kcX6LTlUg7BUL5GWzJyBOhwFD4XcAqomfCfE7cOVmBg7 RXv0SzRbiR+7tUHNc8T7VLP6T41w3r0KO+mQ+aHl4z4Cz9HxCwAPw/MKGDOiyqxg9tgk ED/4nzrFdDbHRHQZL796zzbSHRRQfOFfS4/2UmhDCeZnpoLy3TGouCXdogPZ2Z3Bzi55 lDXQ== X-Gm-Message-State: AElRT7G46QSeAE0EMCsFK7LeckMY1o/+bKvnF13+Nfr6r6pIdfzPtts7 527Q4rtUtwST28Rud+20EYZKJOnRDYVkwZxYDnf+6ZVO X-Google-Smtp-Source: AIpwx4/POQFy54IRtGCco/vRwCNzq6X4sPTaWufcYE49hxHRKcMyywrYHF0YKZ00E2niCChfHJINwfsVcdCyJ8B9m+M= X-Received: by 10.28.46.201 with SMTP id u192mr5099485wmu.161.1522790195276; Tue, 03 Apr 2018 14:16:35 -0700 (PDT) MIME-Version: 1.0 Received: by 10.28.150.150 with HTTP; Tue, 3 Apr 2018 14:16:34 -0700 (PDT) In-Reply-To: References: <5ab4807e.1c69fb81.9b173.0129@mx.google.com> <15F16BD4-EAAE-4F58-8B89-E574C5B2F71A@126.com> From: John Roesler Date: Tue, 3 Apr 2018 16:16:34 -0500 Message-ID: Subject: Re: Gradle strategy for exposing and using public test-utils modules To: dev@kafka.apache.org Content-Type: multipart/alternative; boundary="001a11434130a849f10568f83856" --001a11434130a849f10568f83856 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hello again all, It turns out that the implementation I provided was not correct after all. The issue was gradle tracked the compiled source files included via > compile project(':streams').sourceSets.main.output and included them in the release tarball. Ewen found the issue and corrected the project to use "compileOnly" instead, which uses the dependency to compile the project but does not export the dependency: https://github.com/apache/kafka/pull/4816 Although Ewen's patch does produce the desired output, I think we'd rather have test-utils export a regular dependency on the main streams module. But we can apply the same strategy to allow the streams tests to pull in test-utils as a compile-only dependency: > testCompileOnly project(':streams:test-utils') Here's a PR with the new proposed strategy: https://github.com/apache/kafka/pull/4821. Feel free to comment. For those wishing for a summary, here's the new proposed strategy: project(':streams') { > ... > dependencies { > ... > // this breaks the dependency cycle: testCompileOnly project(':streams:test-utils') > ... > } > ... > } > project(':streams:test-utils') { > ... > dependencies { > compile project(':streams') > ... > } > ... > } > project(':streams:examples') { > ... > dependencies { > compile project(':streams') > ... > testCompile project(':streams:test-utils') > ... > } > ... > } Thanks, -John On Tue, Mar 27, 2018 at 7:06 PM, John Roesler wrote: > Hi again everyone, > > Just for the sake of closure, I think everyone is generally in agreement > with this approach. If concerns arise later on, please let me know! > > Thanks, > -John > > On Fri, Mar 23, 2018 at 12:41 AM, zhenya Sun wrote: > >> +1 >> > =E5=9C=A8 2018=E5=B9=B43=E6=9C=8823=E6=97=A5=EF=BC=8C=E4=B8=8B=E5=8D= =8812:20=EF=BC=8CTed Yu =E5=86=99=E9=81=93=EF=BC=9A >> > >> > +1 >> > -------- Original message --------From: "Matthias J. Sax" < >> matthias@confluent.io> Date: 3/22/18 9:07 PM (GMT-08:00) To: >> dev@kafka.apache.org Subject: Re: Gradle strategy for exposing and using >> public test-utils modules >> > +1 from my side. >> > >> > -Matthias >> > >> > On 3/22/18 5:12 PM, John Roesler wrote: >> >> Yep, I'm super happy with this approach vs. a third module just for t= he >> >> tests. >> >> >> >> For clairty, here's a PR demonstrating the model we're proposing: >> >> https://github.com/apache/kafka/pull/4760 >> >> >> >> Thanks, >> >> -John >> >> >> >> On Thu, Mar 22, 2018 at 6:21 PM, Guozhang Wang >> wrote: >> >> >> >>> I'm +1 to the approach as well across modules that are going to have >> test >> >>> utils artifacts in the future. To me this seems to be a much smaller >> change >> >>> we can make to break the circular dependencies than creating a new >> package >> >>> for our own testing code. >> >>> >> >>> Guozhang >> >>> >> >>> On Thu, Mar 22, 2018 at 1:26 PM, Bill Bejeck >> wrote: >> >>> >> >>>> John, >> >>>> >> >>>> Thanks for the clear, detailed explanation. >> >>>> >> >>>> I'm +1 on what you have proposed. >> >>>> While I agree with you manually pulling in transitive test >> dependencies >> >>> is >> >>>> not ideal, in this case, I think it's worth it to get over the >> circular >> >>>> dependency hurdle and use streams:test-utils ourselves. >> >>>> >> >>>> -Bill >> >>>> >> >>>> On Thu, Mar 22, 2018 at 4:09 PM, John Roesler >> wrote: >> >>>> >> >>>>> Hey everyone, >> >>>>> >> >>>>> In 1.1, kafka-streams adds an artifact called >> >>> 'kafka-streams-test-utils' >> >>>>> (see >> >>>>> https://kafka.apache.org/11/documentation/streams/ >> >>>>> developer-guide/testing.html >> >>>>> ). >> >>>>> >> >>>>> The basic idea is to provide first-class support for testing Kafka >> >>>> Streams >> >>>>> applications. Without that, users were forced to either depend on >> our >> >>>>> internal test artifacts or develop their own test utilities, >> neither of >> >>>>> which is ideal. >> >>>>> >> >>>>> I think it would be great if all our APIs offered a similar module= , >> and >> >>>> it >> >>>>> would all be good if we followed a similar pattern, so I'll descri= be >> >>> the >> >>>>> streams approach along with one challenge we had to overcome: >> >>>>> >> >>>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> >>>>> =3D Project Structure =3D >> >>>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> >>>>> >> >>>>> The directory structure goes: >> >>>>> >> >>>>> kafka/streams/ <- main module code here >> >>>>> /test-utils/ <- test utilities module here >> >>>>> /examples/ <- example usages here >> >>>>> >> >>>>> Likewise, the artifacts are: >> >>>>> >> >>>>> kafka-streams >> >>>>> kafka-streams-test-utils >> >>>>> kafka-streams-examples >> >>>>> >> >>>>> And finally, the Gradle build structure is: >> >>>>> >> >>>>> :streams >> >>>>> :streams:test-utils >> >>>>> :streams:examples >> >>>>> >> >>>>> >> >>>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D >> >>>>> =3D Problem 1: circular build =3D >> >>>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D >> >>>>> >> >>>>> In eat-your-own-dogfood tradition, we wanted to depend on our own >> >>>>> test-utils in our streams tests, but :streams:test-utils (obviousl= y) >> >>>>> depends on :streams already. >> >>>>> >> >>>>> (:streams) <-- (:streams:test-utils) >> >>>>> \---> >> >>>>> >> >>>>> Luckily, Filipe Agapito found a way out of the conundrum ( >> >>>>> https://issues.apache.org/jira/browse/KAFKA-6474? >> >>>>> focusedCommentId=3D16402326&page=3Dcom.atlassian.jira. >> >>>>> plugin.system.issuetabpanels:comment-tabpanel#comment-16402326). >> >>>>> Many thanks to him for this contribution. >> >>>>> >> >>>>> * Add this to the ':streams' definition: >> >>>>> testCompile project(':streams:test-utils') >> .sourceSets.main.output >> >>>>> >> >>>>> * And this to the ':streams:test-utils' definition: >> >>>>> compile project(':streams').sourceSets.main.output >> >>>>> >> >>>>> * And finally (because we also have tests for the examples), add >> this >> >>> to >> >>>>> the ':streams:examples' definition: >> >>>>> testCompile project(':streams:test-utils') >> >>>>> >> >>>>> >> >>>>> >> >>>>> By scoping the dependencies to 'sourceSets.main', we break the >> cyclic >> >>>>> dependency: >> >>>>> >> >>>>> (:streams main) <-- (:streams:test-utils main) >> >>>>> ^ ^ ^ >> >>>>> | / | >> >>>>> | / | >> >>>>> (:streams test) (:streams:test-utils test) >> >>>>> >> >>>>> >> >>>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> >>>>> =3D Problem 2: missing transitive dependencies =3D >> >>>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> >>>>> >> >>>>> Scoping the dependency to source-only skips copying transitive >> library >> >>>>> dependencies into the build & test environment, so we ran into the >> >>>>> following error in our tests for ':streams:test-utils' : >> >>>>> >> >>>>> java.lang.ClassNotFoundException: org.rocksdb.RocksDBException >> >>>>> >> >>>>> This kind of thing is easy to resolve, once you understand why it >> >>>> happens. >> >>>>> We just added this to the :test-utils build definition: >> >>>>> testCompile libs.rocksDBJni >> >>>>> >> >>>>> It's a little unfortunate to have to manually pull in transitive >> >>>>> dependencies for testing, but it's really the only downside of thi= s >> >>>>> approach (so far). >> >>>>> >> >>>>> >> >>>>> >> >>>>> That's about it! This is partly to propose a similar model across >> other >> >>>>> parts of Kafka's API and partly to collect feedback on this >> approach. >> >>>>> >> >>>>> Thoughts? >> >>>>> -John >> >>>>> >> >>>> >> >>> >> >>> >> >>> >> >>> -- >> >>> -- Guozhang >> >>> >> >> >> > >> >> > --001a11434130a849f10568f83856--