Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13080 )
Change subject: [backup] Fix fromMs override option ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/13080/1/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/13080/1/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala@71 PS1, Line 71: full Nit: maybe "previous" here, to indicate that we couldn't find an incremental backup either? I get that we do really need to find a full at the beginning of the chain, but "previous" is similar to what we logged on L64. http://gerrit.cloudera.org:8080/#/c/13080/1/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala File java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala: http://gerrit.cloudera.org:8080/#/c/13080/1/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala@51 PS1, Line 51: def isIncremental: Boolean = { Would this be more useful if it returned a tri-state enum? Something like FULL, INCREMENTAL, or AUTO_DETECT. Then KuduBackup could call it and switch on the result to decide what to do. http://gerrit.cloudera.org:8080/#/c/13080/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/13080/1/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala@103 PS1, Line 103: logs.attach() Nit: mind attaching only for the doBackup call? Makes it a bit easier to figure out what actually needs to be captured. -- To view, visit http://gerrit.cloudera.org:8080/13080 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8a3abc47dd9d1441ba269dfc9405691f79e6615d Gerrit-Change-Number: 13080 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-Comment-Date: Tue, 23 Apr 2019 04:09:35 +0000 Gerrit-HasComments: Yes
