[GitHub] [iceberg] pvary commented on a diff in pull request #6499: AWS, Core, Hive: Fix `checkCommitStatus` when create table commit fails

["pvary (via GitHub)" <gi...@apache.org>]

pvary commented on code in PR #6499:
URL: https://github.com/apache/iceberg/pull/6499#discussion_r1168205368


##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCommits.java:
##########
@@ -320,6 +326,57 @@ public void testAlreadyExistsException() {
         () -> catalog.createTable(TABLE_IDENTIFIER, schema, PartitionSpec.unpartitioned()));
   }
 
+  @Test
+  public void testCreateTableCommitThrowsUnknownException()
+      throws TException, InterruptedException {
+    String namespace = DB_NAME;
+    String tableName = getRandomName();
+    TableIdentifier tableIdentifier = TableIdentifier.of(namespace, tableName);
+
+    HiveTableOperations hiveTableOperations =
+        (HiveTableOperations) catalog.newTableOps(tableIdentifier);
+
+    String tableLocation = catalog.defaultWarehouseLocation(tableIdentifier);
+    TableMetadata metadataV1 = createTableMetadata(tableLocation);
+
+    HiveTableOperations spyOps = spy(hiveTableOperations);
+    failCommitAndThrowException(spyOps);
+
+    Assertions.assertThatThrownBy(() -> spyOps.commit(null, metadataV1))
+        .isInstanceOf(RuntimeException.class)
+        .hasMessageContaining("Datacenter on fire");
+
+    Assert.assertEquals(
+        "One metadata files should exist",
+        1,
+        metadataFileCount(tableLocation + "/writeMetaDataLoc"));

Review Comment:
   Sorry for the late answer!
   Could we use assumptions about the table location instead in the tests? I am afraid that the default behavior is different from the behavior when the location is set, and with this tests we test only the second scenario. OTOH creating a Table without specifically setting the location is the way we use Iceberg in Hive. So it would be good to test that case as well.
   
   Thanks again for taking care of this!



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org