Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8585 )
Change subject: [spark] add 'local-cluster' mode for unit test ...................................................................... Patch Set 1: (3 comments) > I pointed SPARK_HOME at the unpacked release tarball downloaded > from spark.apache.org. Is the source tarball really necessary? Looking at the spark source code, https://github.com/apache/spark/blob/master/launcher/src/main/java/org/apache/spark/launcher/AbstractCommandBuilder.java#L242, (where the failure actually come from), a built source code do seems necessary. http://gerrit.cloudera.org:8080/#/c/8585/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala File java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala: http://gerrit.cloudera.org:8080/#/c/8585/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala@69 PS1, Line 69: testMode.toLowerCase > Can be simplified to I want to do a toLowerCase here, that is why I have the extra null checking. But do you http://gerrit.cloudera.org:8080/#/c/8585/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala@78 PS1, Line 78: conf.setMaster("local[*]") > I think it'd be better to throw an exception here so that it fails when TES Not really, I agree with you we should throw an exception here after thinking more about it. http://gerrit.cloudera.org:8080/#/c/8585/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala@80 PS1, Line 80: val executorLog: String = System.getenv("EXE_LOG_PATH") > These configuration knobs are going to be hard to use, and I don't think th Sure, but we have to add the class path configuration otherwise it will through runtime error. -- To view, visit http://gerrit.cloudera.org:8080/8585 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I07b2daad6f6883eb7425fa7132208818bbe65788 Gerrit-Change-Number: 8585 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Wed, 22 Nov 2017 20:30:31 +0000 Gerrit-HasComments: Yes
