Tony Foerster has posted comments on this change. ( http://gerrit.cloudera.org:8080/10941 )
Change subject: Add error handling to Backup and Restore ...................................................................... Patch Set 8: (8 comments) http://gerrit.cloudera.org:8080/#/c/10941/6/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala File java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala: http://gerrit.cloudera.org:8080/#/c/10941/6/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala@35 PS6, Line 35: private val cause: Throwable = null) > nit: Just None.orNull is a bit unusual. Just null should work. Done http://gerrit.cloudera.org:8080/#/c/10941/6/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala@46 PS6, Line 46: // TODO: Make parallel so each table isn't process serially? > nit: s/vailid/valid Done http://gerrit.cloudera.org:8080/#/c/10941/6/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala@57 PS6, Line 57: // Backup for this timestamp has already been completed. > I think we should only consider the case where getToMs == options.timestamp Done http://gerrit.cloudera.org:8080/#/c/10941/6/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala@111 PS6, Line 111: val out = fs.create(hPath, /* overwrite= */ false) > nit: extra spaces Done http://gerrit.cloudera.org:8080/#/c/10941/6/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala File java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala: http://gerrit.cloudera.org:8080/#/c/10941/6/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala@90 PS6, Line 90: val conf = session.sparkContext.hadoopConfiguration > nit: extra spaces. I think a formatter is aligning the =. This should be fi right it's my formatter... http://gerrit.cloudera.org:8080/#/c/10941/6/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala@103 PS6, Line 103: context: KuduContext): Try[KuduTable] = Try { > nit: explicit return type. ?? This has an explicit return type http://gerrit.cloudera.org:8080/#/c/10941/6/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala@115 PS6, Line 115: def main(args: Array[String]): Unit = { > Where is this used? We definitely don't want the restore process dropping t It was used when removing partial "${tableName}-restore" tables. I've removed that part of the code for now but this was left behind. Fixed. http://gerrit.cloudera.org:8080/#/c/10941/6/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala@136 PS6, Line 136: > nit: s/note/not Done -- To view, visit http://gerrit.cloudera.org:8080/10941 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I104fd604e12b70fcff9beace71ed4ef96b54d691 Gerrit-Change-Number: 10941 Gerrit-PatchSet: 8 Gerrit-Owner: Tony Foerster <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tony Foerster <[email protected]> Gerrit-Comment-Date: Mon, 23 Jul 2018 19:36:21 +0000 Gerrit-HasComments: Yes
