yyanyy commented on a change in pull request #2402:
URL: https://github.com/apache/iceberg/pull/2402#discussion_r605868688



##########
File path: 
aws/src/main/java/org/apache/iceberg/aws/glue/GlueTableOperations.java
##########
@@ -103,14 +105,30 @@ protected void doRefresh() {
   @Override
   protected void doCommit(TableMetadata base, TableMetadata metadata) {
     String newMetadataLocation = writeNewMetadata(metadata, currentVersion() + 
1);
-    boolean exceptionThrown = true;
+    CommitStatus commitStatus = CommitStatus.FAILURE;
+
     try {
       lock(newMetadataLocation);
       Table glueTable = getGlueTable();
       checkMetadataLocation(glueTable, base);
       Map<String, String> properties = prepareProperties(glueTable, 
newMetadataLocation);
-      persistGlueTable(glueTable, properties);
-      exceptionThrown = false;
+
+      try {
+        persistGlueTable(glueTable, properties);
+        commitStatus = CommitStatus.SUCCESS;
+      } catch (Throwable persistFailure) {

Review comment:
       Sounds good, I was thinking if it's a `Error` we will very unlikely be 
able have a success commit before that, so it will be rethrown anyway, but on 
the other hand we probably want to make sure the process to die immediately. 
I'll update L137 to catch runtime exception. 




-- 
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.

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