Jean-Daniel Cryans has posted comments on this change. Change subject: Adding kudu datasource for spark ......................................................................
Patch Set 9: (5 comments) Good stuff, a few important things that should be taken care of as well as some nits. http://gerrit.cloudera.org:8080/#/c/2848/9//COMMIT_MSG Commit Message: Line 8: Can we get a description of what's being added, what incompatible changes have been made, testing that was done, etc? Ideally this patch should also add a section in the release notes: https://github.com/cloudera/kudu/blob/master/docs/release_notes.adoc#rn_0.9.0 http://gerrit.cloudera.org:8080/#/c/2848/9/java/kudu-spark/src/main/scala/org/kududb/spark/kudu/KuduRDD.scala File java/kudu-spark/src/main/scala/org/kududb/spark/kudu/KuduRDD.scala: Line 1: package org.kududb.spark.kudu Missing license. http://gerrit.cloudera.org:8080/#/c/2848/9/java/kudu-spark/src/main/scala/org/kududb/spark/kudu/package.scala File java/kudu-spark/src/main/scala/org/kududb/spark/kudu/package.scala: Line 1: Missing license. http://gerrit.cloudera.org:8080/#/c/2848/9/java/kudu-spark/src/test/scala/org/kududb/spark/kudu/DefaultSourceTest.scala File java/kudu-spark/src/test/scala/org/kududb/spark/kudu/DefaultSourceTest.scala: Line 66: Remove. http://gerrit.cloudera.org:8080/#/c/2848/9/java/kudu-spark/src/test/scala/org/kududb/spark/kudu/TestContext.scala File java/kudu-spark/src/test/scala/org/kududb/spark/kudu/TestContext.scala: Line 113: kuduSession.flush() Doesn't seem like flushing is required since the session's flush mode wasn't changed. -- To view, visit http://gerrit.cloudera.org:8080/2848 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0f91772f58e9eee9de45901866867e9a5014cfbe Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Chris George <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
