ebyhr commented on code in PR #16073:
URL: https://github.com/apache/iceberg/pull/16073#discussion_r3127882104


##########
aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java:
##########
@@ -356,8 +356,13 @@ public boolean dropTable(TableIdentifier identifier, 
boolean purge) {
               .build());
       LOG.info("Successfully dropped table {} from Glue", identifier);
       if (purge && lastMetadata != null) {
-        CatalogUtil.dropTableData(ops.io(), lastMetadata);
-        LOG.info("Glue table {} data purged", identifier);
+        try {
+          CatalogUtil.dropTableData(ops.io(), lastMetadata);
+          LOG.info("Glue table {} data purged", identifier);

Review Comment:
   I was wondering if we could do both (S3 Table check + try-catch) to avoid 
redundant S3 requests and warning logs. I think we should keep the try-catch 
regardless of S3 Table because it may fail for other reasons. 
   
   The "Enumerate all possible URI formats" approach doesn't look 
straightforward. Only adding try-catch looks good to me. 



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