Tony Foerster has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10899 )

Change subject: Test for backup/restore multiple tables
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/10899/1/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
File 
java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala:

http://gerrit.cloudera.org:8080/#/c/10899/1/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala@97
PS1, Line 97: num
> Nit: maybe rename this to "numRows" to assign it some kind of semantic mean
Done


http://gerrit.cloudera.org:8080/#/c/10899/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/10899/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala@111
PS1, Line 111:   val tableOptions: CreateTableOptions = {
             :     val bottom = schema.newPartialRow() // Unbounded.
             :     val middle = schema.newPartialRow()
             :     middle.addInt("key", 50)
> Could you parameterize this so that createTable() can also be used to creat
I couldn't parameterize it without ending up back at the KuduClient createTable 
signature
      kuduClient.createTable(tableName, schema, tableOptions)

I made the CreateTableOptions public and just used that.


http://gerrit.cloudera.org:8080/#/c/10899/1/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala@137
PS1, Line 137: targetTable: Kud
> This is now shadowing the existing class-level 'table' variable; could you
Done



--
To view, visit http://gerrit.cloudera.org:8080/10899
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I439e0d1c6904395ad382e3f059846b3b03a79af4
Gerrit-Change-Number: 10899
Gerrit-PatchSet: 2
Gerrit-Owner: Tony Foerster <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tony Foerster <[email protected]>
Gerrit-Comment-Date: Thu, 12 Jul 2018 13:53:25 +0000
Gerrit-HasComments: Yes

Reply via email to