Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/11788 )
Change subject: [examples] Add basic Spark example written in Scala ...................................................................... Patch Set 4: (11 comments) http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/README.adoc File examples/scala/spark-example/README.adoc: http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/README.adoc@43 PS1, Line 43: To specify a value at execution time, you'll specify the parameter name in the '-D<name>' format. For example, to set a different set of masters for the Kudu cluster from the command line and use a custom table name, set the property `KuduMasters` to a CSV of the master addresses in the form `host:port` and add a table name value, as shown: > I didn't see any default listed for this value - it's inclusion is only due I meant the default you are using in your code. For example, by default you set SPARK_MASTER to local if it isn't provided. http://gerrit.cloudera.org:8080/#/c/11788/4/examples/scala/spark-example/README.adoc File examples/scala/spark-example/README.adoc: http://gerrit.cloudera.org:8080/#/c/11788/4/examples/scala/spark-example/README.adoc@43 PS4, Line 43: To specify a value at execution time, you'll specify the parameter name in the '-D<name>' format. For example, to set a different set of masters for the Kudu cluster from the command line and use a custom table name, set the property `KuduMasters` to a CSV of the master addresses in the form `host:port` and add a table name value, as shown: Nit: line breaks. http://gerrit.cloudera.org:8080/#/c/11788/4/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala File examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala: http://gerrit.cloudera.org:8080/#/c/11788/4/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@16 PS4, Line 16: val KuduMasters: String = System.getProperty("KuduMasters","kudu.master1:7051,kudu.master2:7051,kudu.master3:7051") //kudu master address list Can we use localhost:7051 as the default? http://gerrit.cloudera.org:8080/#/c/11788/4/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@22 PS4, Line 22: //defining a class that we'll use to insert data into the table Nit: periods at the end of comments. http://gerrit.cloudera.org:8080/#/c/11788/4/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@27 PS4, Line 27: val logger = LoggerFactory.getLogger(SparkExample.getClass) nit: this can be defined at the top of the object. http://gerrit.cloudera.org:8080/#/c/11788/4/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@34 PS4, Line 34: import spark.implicits._ Can you try moving this? It should be fine to be with the other imports. http://gerrit.cloudera.org:8080/#/c/11788/4/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@39 PS4, Line 39: List( nit: This indentation seams level looks like it is using 4 spaces. http://gerrit.cloudera.org:8080/#/c/11788/4/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@40 PS4, Line 40: StructField(IdCol,IntegerType,false), nit: space after commas. http://gerrit.cloudera.org:8080/#/c/11788/4/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@48 PS4, Line 48: kc.createTable(TableName, schema, Seq(IdCol), new CreateTableOptions().setNumReplicas(3).addHashPartitions(List(IdCol).asJava, 3)) This still has a replication factor of 3. http://gerrit.cloudera.org:8080/#/c/11788/4/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@71 PS4, Line 71: val upsertUsers = Array(User("newUserA", 1234), User("userC", 7777)) To show the value of an upsert, should we re-use a key that was already inserted? http://gerrit.cloudera.org:8080/#/c/11788/4/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@84 PS4, Line 84: finally try { just finally? -- To view, visit http://gerrit.cloudera.org:8080/11788 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f Gerrit-Change-Number: 11788 Gerrit-PatchSet: 4 Gerrit-Owner: Mitch Barnett <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mitch Barnett <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Fri, 26 Oct 2018 13:59:50 +0000 Gerrit-HasComments: Yes
