Dan Burkert has posted comments on this change. Change subject: Spark connectors for Kudu ......................................................................
Patch Set 4: (5 comments) http://gerrit.cloudera.org:8080/#/c/1788/4/java/kudu-spark/pom.xml File java/kudu-spark/pom.xml: Line 92: <artifactId>hadoop-client</artifactId> > hadoop-client includes stuff like the HDFS client which we shouldn't need. Done http://gerrit.cloudera.org:8080/#/c/1788/4/java/kudu-spark/src/main/scala/org/kududb/spark/DefaultSource.scala File java/kudu-spark/src/main/scala/org/kududb/spark/DefaultSource.scala: Line 90: kuduSchema.getColumns.asScala.map({ c => c.getName -> c }).toMap > this is my scala inexperience talking, but I find this hard to know whether Done Line 129: def getKuduValue(row: RowResult, columnName: String): Any = { > can this be private? Done http://gerrit.cloudera.org:8080/#/c/1788/4/java/kudu-spark/src/main/scala/org/kududb/spark/KuduContext.scala File java/kudu-spark/src/main/scala/org/kududb/spark/KuduContext.scala: Line 120: * > nit: extra space on this line and the next Done Line 144: f(it, syncClient, asyncClient) > I think this causes both of the clients to end up evaluated, which construc This is a private method not used anywhere, so I've gone ahead and removed it. Your comments do apply to kuduMapPartition which is not dead, though. The design of this class is really poor, and not something that we will want to support in the long term. If we're going to change it, I would just vote to axe most of the methods all together. They don't use any public APIs, so its not something that clients couldn't do on their own. -- To view, visit http://gerrit.cloudera.org:8080/1788 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic187513ef9724d50024f7401d7ecd19d53554245 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
