szehon-ho commented on code in PR #5622:
URL: https://github.com/apache/iceberg/pull/5622#discussion_r980598504


##########
spark/v3.0/spark/src/main/java/org/apache/iceberg/spark/actions/BaseMigrateTableSparkAction.java:
##########
@@ -221,4 +230,12 @@ private void restoreSourceTable() {
           e);
     }
   }
+
+  private void dropBackupTable() {
+    try {
+      destCatalog().dropTable(backupIdent);
+    } catch (Exception e) {
+      LOG.error("Cannot drop the backup table {} after migrate is completed.", 
backupIdent, e);

Review Comment:
   Nit: grammar error here (should be- after + noun).  How about,  'Failed to 
drop the backup table {} after the migration'



##########
.palantir/revapi.yml:
##########
@@ -3,12 +3,19 @@ versionOverrides:
 acceptedBreaks:
   apache-iceberg-0.14.0:
     org.apache.iceberg:iceberg-api:
+    - code: "java.class.defaultSerializationChanged"

Review Comment:
   Why is partition key complaining here?  Can we not add this one?



##########
api/src/main/java/org/apache/iceberg/actions/MigrateTable.java:
##########
@@ -41,6 +41,14 @@ public interface MigrateTable extends Action<MigrateTable, 
MigrateTable.Result>
    */
   MigrateTable tableProperty(String name, String value);
 
+  /**
+   * Sets the dropBackup property to drop backup table after a successful 
table Migration
+   *
+   * @param dropBackup flag to remove backup table

Review Comment:
   Can be more descriptive, maybe "if true, removes the backup table"



##########
api/src/main/java/org/apache/iceberg/actions/MigrateTable.java:
##########
@@ -41,6 +41,14 @@ public interface MigrateTable extends Action<MigrateTable, 
MigrateTable.Result>
    */
   MigrateTable tableProperty(String name, String value);
 
+  /**
+   * Sets the dropBackup property to drop backup table after a successful 
table Migration

Review Comment:
   Capitlization looks a bit weird, let's revert Migration => migration?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to