You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Grant Henke (Code Review)" <ge...@cloudera.org> on 2018/07/10 21:56:49 UTC
[kudu-CR] WIP: support for running Java tests in dist-test
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10907
Change subject: WIP: support for running Java tests in dist-test
......................................................................
WIP: support for running Java tests in dist-test
Probably not worth a serious review yet, since this has a few failing tests yet. However, it maybe worth glancing over the "big idea".
Sample usage:
$ python build-support/dist_test.py java run-all
$ python build-support/dist_test.py java loop --num-instances 10 *AsyncKuduSession*
Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
---
M build-support/dist_test.py
M build-support/run-test.sh
M build-support/run_dist_test.py
M java/build.gradle
M java/buildSrc/build.gradle
A java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java
6 files changed, 434 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/10907/1
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
[kudu-CR] Add support for running Java tests in dist-test
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10907 )
Change subject: Add support for running Java tests in dist-test
......................................................................
Patch Set 5:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/10907/5/build-support/run_dist_test.py
File build-support/run_dist_test.py:
http://gerrit.cloudera.org:8080/#/c/10907/5/build-support/run_dist_test.py@42
PS5, Line 42: read
> use readlines here instead of having to split('\n') below
Done
http://gerrit.cloudera.org:8080/#/c/10907/5/build-support/run_dist_test.py@44
PS5, Line 44: JAVA_CANDIDATES = [x for x in JAVA_CANDIDATES if not x.startswith("#")]
> maybe just combine the 'if not' part into the above comprehension?
Done
http://gerrit.cloudera.org:8080/#/c/10907/5/build-support/run_dist_test.py@101
PS5, Line 101: print >>sys.stderr, "found JAVA_HOME: " + x
> if you use logging, you'll get timestamps, plus be python-3 compatible. Or
Done
http://gerrit.cloudera.org:8080/#/c/10907/5/build-support/run_dist_test.py@112
PS5, Line 112: p.add_option("--test-type", dest="test_type", action="store",
> maybe test_language is better?
Done
http://gerrit.cloudera.org:8080/#/c/10907/5/cmake_modules/FindJavaHome.cmake
File cmake_modules/FindJavaHome.cmake:
http://gerrit.cloudera.org:8080/#/c/10907/5/cmake_modules/FindJavaHome.cmake@29
PS5, Line 29: #if (DEFINED ENV{JAVA_HOME})
> why's this commented out?
oops. I needed to test this on a mac so I commented it out.
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 17 Jul 2018 02:45:16 +0000
Gerrit-HasComments: Yes
[kudu-CR] Add support for running Java tests in dist-test
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has removed a vote on this change.
Change subject: Add support for running Java tests in dist-test
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Add support for running Java tests in dist-test
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10907 )
Change subject: Add support for running Java tests in dist-test
......................................................................
Add support for running Java tests in dist-test
Adds support for running Java tests in dist-test by:
- Providing a way to gather the c++ “base”
dependencies for the Java test in dist_test.py
- Adding a Java subparser to dist_test.py that can
run or loop Java tests.
- Adding a task to the Gradle build that generates
the needed .isolate and .gen.json files. This task is
called from dist_test.py.
- Adjusting run_dist_test.py to find the Java binaries
and run Java tests.
Sample usage:
$ python build-support/dist_test.py java run-all
$ python build-support/dist_test.py java loop --num-instances 10 *AsyncKuduSession*
Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Reviewed-on: http://gerrit.cloudera.org:8080/10907
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Grant Henke <gr...@apache.org>
---
M build-support/dist_test.py
A build-support/java-home-candidates.txt
M build-support/run_dist_test.py
M cmake_modules/FindJavaHome.cmake
M java/build.gradle
M java/buildSrc/build.gradle
A java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java
7 files changed, 468 insertions(+), 52 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Grant Henke: Verified
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 8
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Add support for running Java tests in dist-test
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Adar Dembo, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10907
to look at the new patch set (#4).
Change subject: Add support for running Java tests in dist-test
......................................................................
Add support for running Java tests in dist-test
Adds support for running Java tests in dist-test by:
- Providing a way to gather the c++ “base”
dependencies for the Java test in dist_test.py
- Adding a Java subparser to dist_test.py that can
run or loop Java tests.
- Adding a task to the Gradle build that generates
the needed .isolate and .gen.json files. This task is
called from dist_test.py.
- Adjusting run_dist_test.py to find the Java binaries
and run Java tests.
Sample usage:
$ python build-support/dist_test.py java run-all
$ python build-support/dist_test.py java loop --num-instances 10 *AsyncKuduSession*
Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
---
M build-support/dist_test.py
M build-support/run-test.sh
M build-support/run_dist_test.py
M java/build.gradle
M java/buildSrc/build.gradle
A java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java
6 files changed, 446 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/10907/4
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Add support for running Java tests in dist-test
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10907 )
Change subject: Add support for running Java tests in dist-test
......................................................................
Patch Set 6: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 17 Jul 2018 03:26:13 +0000
Gerrit-HasComments: No
[kudu-CR] Add support for running Java tests in dist-test
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has removed a vote on this change.
Change subject: Add support for running Java tests in dist-test
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Add support for running Java tests in dist-test
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10907
to look at the new patch set (#3).
Change subject: Add support for running Java tests in dist-test
......................................................................
Add support for running Java tests in dist-test
Adds support for running Java tests in dist-test by:
- Providing a way to gather the c++ “base”
dependencies for the Java test in dist_test.py
- Adding a Java subparser to dist_test.py that can
run or loop Java tests.
- Adding a task to the Gradle build that generates
the needed .isolate and .gen.json files. This task is
called from dist_test.py.
- Adjusting run_dist_test.py to find the Java binaries
and run Java tests.
Sample usage:
$ python build-support/dist_test.py java run-all
$ python build-support/dist_test.py java loop --num-instances 10 *AsyncKuduSession*
Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
---
M build-support/dist_test.py
M build-support/run-test.sh
M build-support/run_dist_test.py
M java/build.gradle
M java/buildSrc/build.gradle
A java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java
6 files changed, 439 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/10907/3
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Add support for running Java tests in dist-test
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10907 )
Change subject: Add support for running Java tests in dist-test
......................................................................
Patch Set 6:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/10907/6/build-support/java-home-candidates.txt
File build-support/java-home-candidates.txt:
PS6:
> Should probably add a license header and a brief description of what this f
Done
http://gerrit.cloudera.org:8080/#/c/10907/3/build-support/run_dist_test.py
File build-support/run_dist_test.py:
http://gerrit.cloudera.org:8080/#/c/10907/3/build-support/run_dist_test.py@179
PS3, Line 179: if __name__ == "__main__":
> Don't the Java tests always use the binaries from build/latest?
The `latest` symlink is not marked as a dependency directory so it's not sent to each dist-test task. I am not sure we would want to either.
http://gerrit.cloudera.org:8080/#/c/10907/3/java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java
File java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java:
http://gerrit.cloudera.org:8080/#/c/10907/3/java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java@73
PS3, Line 73: public void addTestTask(Test t) {
> OK, could you add a comment explaining that?
Done
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 17 Jul 2018 15:28:08 +0000
Gerrit-HasComments: Yes
[kudu-CR] Add support for running Java tests in dist-test
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10907 )
Change subject: Add support for running Java tests in dist-test
......................................................................
Patch Set 3:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/10907/3/build-support/run_dist_test.py
File build-support/run_dist_test.py:
http://gerrit.cloudera.org:8080/#/c/10907/3/build-support/run_dist_test.py@178
PS3, Line 178: os.path.join(ROOT, "build/lib/")] +
> What is this directory? Isn't the build tree comprised of build/debug, buil
This change is a carry over from Todds initial patch. I am not sure how build/lib is populated.
http://gerrit.cloudera.org:8080/#/c/10907/3/build-support/run_dist_test.py@181
PS3, Line 181: # GTEST_OUTPUT must be canonicalized and have a trailing slash for gtest to
: # properly interpret it as a directory.
: test_logdir = os.path.abspath(os.path.join(ROOT, "build/test-logs"))
: env['GTEST_OUTPUT'] = 'xml:' + test_logdir + '/'
: env['TEST_LOGDIR'] = test_logdir
> This is already set up for C++ tests in run-test.sh. Why do we need to dupl
The Java tests don't use run-test.sh. I didn't make this change, It came from Todds original patch. But I think he simplified dis-test runs to use build/test-logs instead of build/<test-type>/test-log because we don't need to worry about test type collisions in dist-test.
http://gerrit.cloudera.org:8080/#/c/10907/3/java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java
File java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java:
http://gerrit.cloudera.org:8080/#/c/10907/3/java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java@65
PS3, Line 65:
: String distTestBin = getProject().getRootDir() + "/../build-support/dist_test.py";
:
: @OutputDirectory
: File outputDir = new File(getProject().getBuildDir(), "dist-test");
:
: List<Test> testTasks = Lists.newArrayList();
> private?
Done
http://gerrit.cloudera.org:8080/#/c/10907/3/java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java@73
PS3, Line 73: public void testTask(Test t) {
> Maybe addTestTask?
This is called in the main build.gradle file. It doesn't need an annotation to be called because it public.
http://gerrit.cloudera.org:8080/#/c/10907/3/java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java@91
PS3, Line 91: if (fc == null) {
: fc = t.getCandidateClassFiles();
: } else {
: fc = fc.plus(t.getCandidateClassFiles());
: }
> Maybe clearer as:
I found a way to initialize an empty FileCollection which is more clear.
http://gerrit.cloudera.org:8080/#/c/10907/3/java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java@156
PS3, Line 156: // This hack replaces jars from the gradle cache in ~/.gradle/caches/...
: // with jars copied under the project build directory.
> As worded it sort of sounds like the hack will overwrite the files in ~/.gr
Done
http://gerrit.cloudera.org:8080/#/c/10907/3/java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java@255
PS3, Line 255:
> Can you add class comments to these to explain why the layout is the way it
Done
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 16 Jul 2018 16:17:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP: support for running Java tests in dist-test
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10907
to look at the new patch set (#2).
Change subject: WIP: support for running Java tests in dist-test
......................................................................
WIP: support for running Java tests in dist-test
Probably not worth a serious review yet, since this has a
few failing tests. However, it maybe worth glancing over
the "big idea".
Sample usage:
$ python build-support/dist_test.py java run-all
$ python build-support/dist_test.py java loop --num-instances 10 *AsyncKuduSession*
Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
---
M build-support/dist_test.py
M build-support/run-test.sh
M build-support/run_dist_test.py
M java/build.gradle
M java/buildSrc/build.gradle
A java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java
6 files changed, 434 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/10907/2
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Add support for running Java tests in dist-test
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has removed a vote on this change.
Change subject: Add support for running Java tests in dist-test
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Add support for running Java tests in dist-test
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Adar Dembo, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10907
to look at the new patch set (#7).
Change subject: Add support for running Java tests in dist-test
......................................................................
Add support for running Java tests in dist-test
Adds support for running Java tests in dist-test by:
- Providing a way to gather the c++ “base”
dependencies for the Java test in dist_test.py
- Adding a Java subparser to dist_test.py that can
run or loop Java tests.
- Adding a task to the Gradle build that generates
the needed .isolate and .gen.json files. This task is
called from dist_test.py.
- Adjusting run_dist_test.py to find the Java binaries
and run Java tests.
Sample usage:
$ python build-support/dist_test.py java run-all
$ python build-support/dist_test.py java loop --num-instances 10 *AsyncKuduSession*
Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
---
M build-support/dist_test.py
A build-support/java-home-candidates.txt
M build-support/run_dist_test.py
M cmake_modules/FindJavaHome.cmake
M java/build.gradle
M java/buildSrc/build.gradle
A java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java
7 files changed, 468 insertions(+), 52 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/10907/7
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Add support for running Java tests in dist-test
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10907 )
Change subject: Add support for running Java tests in dist-test
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 17 Jul 2018 21:39:52 +0000
Gerrit-HasComments: No
[kudu-CR] Add support for running Java tests in dist-test
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10907 )
Change subject: Add support for running Java tests in dist-test
......................................................................
Patch Set 3:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/10907/3/build-support/run_dist_test.py
File build-support/run_dist_test.py:
http://gerrit.cloudera.org:8080/#/c/10907/3/build-support/run_dist_test.py@41
PS3, Line 41: JAVA_CANDIDATES = """
This is cribbed from cmake_modules/FindJavaHome.cmake, right? Can we share the list in a common data file?
http://gerrit.cloudera.org:8080/#/c/10907/3/build-support/run_dist_test.py@178
PS3, Line 178: os.path.join(ROOT, "build/lib/")] +
What is this directory? Isn't the build tree comprised of build/debug, build/asan, build/tsan, build/latest, etc?
http://gerrit.cloudera.org:8080/#/c/10907/3/build-support/run_dist_test.py@179
PS3, Line 179: glob.glob(os.path.abspath((os.path.join(ROOT, "build/*/lib")))))
Why do we need this glob?
http://gerrit.cloudera.org:8080/#/c/10907/3/build-support/run_dist_test.py@181
PS3, Line 181: # GTEST_OUTPUT must be canonicalized and have a trailing slash for gtest to
: # properly interpret it as a directory.
: test_logdir = os.path.abspath(os.path.join(ROOT, "build/test-logs"))
: env['GTEST_OUTPUT'] = 'xml:' + test_logdir + '/'
: env['TEST_LOGDIR'] = test_logdir
This is already set up for C++ tests in run-test.sh. Why do we need to duplicate it here?
Is it because C++ tests will use build/<build_type>/test-logs as their log directory? If so, why do we need to override that?
http://gerrit.cloudera.org:8080/#/c/10907/3/java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java
File java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java:
http://gerrit.cloudera.org:8080/#/c/10907/3/java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java@65
PS3, Line 65:
: String distTestBin = getProject().getRootDir() + "/../build-support/dist_test.py";
:
: @OutputDirectory
: File outputDir = new File(getProject().getBuildDir(), "dist-test");
:
: List<Test> testTasks = Lists.newArrayList();
private?
http://gerrit.cloudera.org:8080/#/c/10907/3/java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java@73
PS3, Line 73: public void testTask(Test t) {
Maybe addTestTask?
Also, where is this called? If it's used by Gradle I'm surprised it doesn't need some kind of annotation. Maybe a comment would help?
http://gerrit.cloudera.org:8080/#/c/10907/3/java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java@91
PS3, Line 91: if (fc == null) {
: fc = t.getCandidateClassFiles();
: } else {
: fc = fc.plus(t.getCandidateClassFiles());
: }
Maybe clearer as:
FileCollection tfc = t.GetCandidateClassFiles();
fc = (fc == null) ? tfc : fc.plus(tfc);
http://gerrit.cloudera.org:8080/#/c/10907/3/java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java@156
PS3, Line 156: // This hack replaces jars from the gradle cache in ~/.gradle/caches/...
: // with jars copied under the project build directory.
As worded it sort of sounds like the hack will overwrite the files in ~/.gradle/caches. Can you reword?
http://gerrit.cloudera.org:8080/#/c/10907/3/java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java@255
PS3, Line 255:
Can you add class comments to these to explain why the layout is the way it is?
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 12 Jul 2018 17:13:01 +0000
Gerrit-HasComments: Yes
[kudu-CR] Add support for running Java tests in dist-test
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10907 )
Change subject: Add support for running Java tests in dist-test
......................................................................
Patch Set 5:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/10907/3/build-support/run_dist_test.py
File build-support/run_dist_test.py:
http://gerrit.cloudera.org:8080/#/c/10907/3/build-support/run_dist_test.py@41
PS3, Line 41: with open(os.path.join(ROOT, "build-support", "java-home-candidates.txt"), 'r') as candidates:
> This is cribbed from cmake_modules/FindJavaHome.cmake, right? Can we share
Done
http://gerrit.cloudera.org:8080/#/c/10907/3/build-support/run_dist_test.py@178
PS3, Line 178: main()
> This change is a carry over from Todds initial patch. I am not sure how bui
I removed this.
http://gerrit.cloudera.org:8080/#/c/10907/3/build-support/run_dist_test.py@179
PS3, Line 179:
> Why do we need this glob?
We Glob because the Java tests can't infer which build type we are using.
http://gerrit.cloudera.org:8080/#/c/10907/3/build-support/run_dist_test.py@181
PS3, Line 181:
:
:
:
:
> The Java tests don't use run-test.sh. I didn't make this change, It came fr
I updated my patch to remove this and use the existing structure.
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 16 Jul 2018 22:15:40 +0000
Gerrit-HasComments: Yes
[kudu-CR] Add support for running Java tests in dist-test
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has removed a vote on this change.
Change subject: Add support for running Java tests in dist-test
......................................................................
Removed Code-Review+1 by Grant Henke <gr...@apache.org>
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Add support for running Java tests in dist-test
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10907 )
Change subject: Add support for running Java tests in dist-test
......................................................................
Patch Set 7: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 18 Jul 2018 14:57:01 +0000
Gerrit-HasComments: No
[kudu-CR] Add support for running Java tests in dist-test
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10907 )
Change subject: Add support for running Java tests in dist-test
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 17 Jul 2018 01:37:58 +0000
Gerrit-HasComments: No
[kudu-CR] Add support for running Java tests in dist-test
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Adar Dembo, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10907
to look at the new patch set (#5).
Change subject: Add support for running Java tests in dist-test
......................................................................
Add support for running Java tests in dist-test
Adds support for running Java tests in dist-test by:
- Providing a way to gather the c++ “base”
dependencies for the Java test in dist_test.py
- Adding a Java subparser to dist_test.py that can
run or loop Java tests.
- Adding a task to the Gradle build that generates
the needed .isolate and .gen.json files. This task is
called from dist_test.py.
- Adjusting run_dist_test.py to find the Java binaries
and run Java tests.
Sample usage:
$ python build-support/dist_test.py java run-all
$ python build-support/dist_test.py java loop --num-instances 10 *AsyncKuduSession*
Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
---
M build-support/dist_test.py
A build-support/java-home-candidates.txt
M build-support/run_dist_test.py
M cmake_modules/FindJavaHome.cmake
M java/build.gradle
M java/buildSrc/build.gradle
A java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java
7 files changed, 466 insertions(+), 73 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/10907/5
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Add support for running Java tests in dist-test
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Adar Dembo, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10907
to look at the new patch set (#6).
Change subject: Add support for running Java tests in dist-test
......................................................................
Add support for running Java tests in dist-test
Adds support for running Java tests in dist-test by:
- Providing a way to gather the c++ “base”
dependencies for the Java test in dist_test.py
- Adding a Java subparser to dist_test.py that can
run or loop Java tests.
- Adding a task to the Gradle build that generates
the needed .isolate and .gen.json files. This task is
called from dist_test.py.
- Adjusting run_dist_test.py to find the Java binaries
and run Java tests.
Sample usage:
$ python build-support/dist_test.py java run-all
$ python build-support/dist_test.py java loop --num-instances 10 *AsyncKuduSession*
Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
---
M build-support/dist_test.py
A build-support/java-home-candidates.txt
M build-support/run_dist_test.py
M cmake_modules/FindJavaHome.cmake
M java/build.gradle
M java/buildSrc/build.gradle
A java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java
7 files changed, 446 insertions(+), 52 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/10907/6
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Add support for running Java tests in dist-test
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10907 )
Change subject: Add support for running Java tests in dist-test
......................................................................
Patch Set 5:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/10907/5/build-support/run_dist_test.py
File build-support/run_dist_test.py:
http://gerrit.cloudera.org:8080/#/c/10907/5/build-support/run_dist_test.py@42
PS5, Line 42: read
use readlines here instead of having to split('\n') below
http://gerrit.cloudera.org:8080/#/c/10907/5/build-support/run_dist_test.py@44
PS5, Line 44: JAVA_CANDIDATES = [x for x in JAVA_CANDIDATES if not x.startswith("#")]
maybe just combine the 'if not' part into the above comprehension?
Mind adding the following to make sure that people don't try to put trailing comments into their list of paths?
for c in JAVA_CANDIDATES:
assert '#' not in c
http://gerrit.cloudera.org:8080/#/c/10907/5/build-support/run_dist_test.py@101
PS5, Line 101: print >>sys.stderr, "found JAVA_HOME: " + x
if you use logging, you'll get timestamps, plus be python-3 compatible. Or just use the print statement with () -- there is some "future" import you have to do to do that, though
http://gerrit.cloudera.org:8080/#/c/10907/5/build-support/run_dist_test.py@112
PS5, Line 112: p.add_option("--test-type", dest="test_type", action="store",
maybe test_language is better?
http://gerrit.cloudera.org:8080/#/c/10907/5/cmake_modules/FindJavaHome.cmake
File cmake_modules/FindJavaHome.cmake:
http://gerrit.cloudera.org:8080/#/c/10907/5/cmake_modules/FindJavaHome.cmake@29
PS5, Line 29: #if (DEFINED ENV{JAVA_HOME})
why's this commented out?
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 17 Jul 2018 01:43:15 +0000
Gerrit-HasComments: Yes
[kudu-CR] Add support for running Java tests in dist-test
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10907 )
Change subject: Add support for running Java tests in dist-test
......................................................................
Patch Set 6: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 17 Jul 2018 03:26:21 +0000
Gerrit-HasComments: No
[kudu-CR] Add support for running Java tests in dist-test
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10907 )
Change subject: Add support for running Java tests in dist-test
......................................................................
Patch Set 6:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/10907/6/build-support/java-home-candidates.txt
File build-support/java-home-candidates.txt:
PS6:
Should probably add a license header and a brief description of what this file does (and that order is important, and anything else that may matter).
http://gerrit.cloudera.org:8080/#/c/10907/3/build-support/run_dist_test.py
File build-support/run_dist_test.py:
http://gerrit.cloudera.org:8080/#/c/10907/3/build-support/run_dist_test.py@179
PS3, Line 179: if __name__ == "__main__":
> We Glob because the Java tests can't infer which build type we are using.
Don't the Java tests always use the binaries from build/latest?
http://gerrit.cloudera.org:8080/#/c/10907/3/java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java
File java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java:
http://gerrit.cloudera.org:8080/#/c/10907/3/java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java@73
PS3, Line 73: public void addTestTask(Test t) {
> This is called in the main build.gradle file. It doesn't need an annotation
OK, could you add a comment explaining that?
--
To view, visit http://gerrit.cloudera.org:8080/10907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557
Gerrit-Change-Number: 10907
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 17 Jul 2018 04:46:57 +0000
Gerrit-HasComments: Yes