bharos commented on code in PR #10898:
URL: https://github.com/apache/gravitino/pull/10898#discussion_r3164205508


##########
iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestCatalogWrapperForREST.java:
##########
@@ -175,6 +188,62 @@ void testConstructorDoesNotLoadCatalogEagerly() {
     }
   }
 
+  @Test
+  void testStageTableCreateWithLocationIncludesFileIoProperties() throws 
Exception {
+    Catalog catalog = mock(Catalog.class);
+    Catalog.TableBuilder tableBuilder = mock(Catalog.TableBuilder.class);
+    Transaction transaction = mock(Transaction.class);
+    Table table = mock(Table.class);
+    FileIO fileIO = mock(FileIO.class);
+    when(catalog.buildTable(any(TableIdentifier.class), 
any())).thenReturn(tableBuilder);
+    when(tableBuilder.withPartitionSpec(any())).thenReturn(tableBuilder);
+    when(tableBuilder.withSortOrder(any())).thenReturn(tableBuilder);
+    when(tableBuilder.withProperties(anyMap())).thenReturn(tableBuilder);
+    
when(tableBuilder.withLocation("s3://bucket/warehouse/table")).thenReturn(tableBuilder);
+    when(tableBuilder.createTransaction()).thenReturn(transaction);
+    when(transaction.table()).thenReturn(table);
+    when(table.io()).thenReturn(fileIO);
+    when(table.location()).thenReturn("s3://bucket/warehouse/table");
+    when(fileIO.properties())
+        .thenReturn(
+            ImmutableMap.of(
+                IcebergConstants.IO_IMPL,
+                "org.apache.iceberg.aws.s3.S3FileIO",
+                IcebergConstants.ICEBERG_S3_ENDPOINT,
+                "http://localhost:9000";));
+
+    IcebergConfig config =
+        new IcebergConfig(
+            ImmutableMap.of(
+                IcebergConstants.CATALOG_BACKEND,
+                "memory",
+                IcebergConstants.WAREHOUSE,
+                "/tmp/warehouse"));
+    CatalogWrapperForREST wrapper = new StaticCatalogWrapperForREST("test", 
config, catalog);
+
+    Schema schema = new Schema(Types.NestedField.required(1, "id", 
Types.IntegerType.get()));
+    CreateTableRequest request =
+        CreateTableRequest.builder()
+            .withName("tbl")
+            .withSchema(schema)
+            .withLocation("s3://bucket/warehouse/table")
+            .stageCreate()
+            .build();
+
+    Method method =
+        CatalogWrapperForREST.class.getDeclaredMethod(
+            "stageTableCreateInternal", Namespace.class, 
CreateTableRequest.class);
+    method.setAccessible(true);

Review Comment:
   Do we need to call the internal method here using reflection.
   Can we call the public entry point createTable(Namespace, 
CreateTableRequest, boolean) with request.stageCreate() == true, which 
naturally routes to stageTableCreateInternal



##########
iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestCatalogWrapperForREST.java:
##########
@@ -175,6 +188,62 @@ void testConstructorDoesNotLoadCatalogEagerly() {
     }
   }
 
+  @Test
+  void testStageTableCreateWithLocationIncludesFileIoProperties() throws 
Exception {

Review Comment:
   Shall we add unit test for the location == null branch as well



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

Reply via email to