Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/13405 )
Change subject: [backup] Add a tool to clean up old backups ...................................................................... Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/13405/3/java/kudu-backup-tools/src/main/scala/org/apache/kudu/backup/KuduBackupCleaner.scala File java/kudu-backup-tools/src/main/scala/org/apache/kudu/backup/KuduBackupCleaner.scala: http://gerrit.cloudera.org:8080/#/c/13405/3/java/kudu-backup-tools/src/main/scala/org/apache/kudu/backup/KuduBackupCleaner.scala@108 PS3, Line 108: options.tables.foreach { tableName => this breaks the options.tables.isEmpty case http://gerrit.cloudera.org:8080/#/c/13405/2/java/kudu-backup-tools/src/test/scala/org/apache/kudu/backup/TestKuduBackupCleaner.scala File java/kudu-backup-tools/src/test/scala/org/apache/kudu/backup/TestKuduBackupCleaner.scala: http://gerrit.cloudera.org:8080/#/c/13405/2/java/kudu-backup-tools/src/test/scala/org/apache/kudu/backup/TestKuduBackupCleaner.scala@118 PS2, Line 118: // Finally, add a third path which is not the restore path but which has backups that are old > Well, we actually sort of can't because backup dirs are named using only th That's a good point. Now that we have CLI tools, it seems like we should change our naming convention, but that is a separate issue. Having divergent descendant paths is something that we can test and while the code is currently written to avoid deleting anything in the restore path, it would be good to have test coverage that breaks if anyone ever messes up that logic in the future. -- To view, visit http://gerrit.cloudera.org:8080/13405 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4a4c4dde10ec3c3c9b2233ee2f7b13c46e963e39 Gerrit-Change-Number: 13405 Gerrit-PatchSet: 3 Gerrit-Owner: Will Berkeley <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Tue, 28 May 2019 17:53:57 +0000 Gerrit-HasComments: Yes
