liamzwbao commented on code in PR #1126:
URL: https://github.com/apache/polaris/pull/1126#discussion_r2003607988


##########
integration-tests/src/main/java/org/apache/polaris/service/it/env/IcebergHelper.java:
##########
@@ -53,11 +54,11 @@ public static RESTCatalog restCatalog(
             .put(
                 org.apache.iceberg.CatalogProperties.FILE_IO_IMPL,
                 "org.apache.iceberg.inmemory.InMemoryFileIO")
-            .put("warehouse", catalog)
+            .put("warehouse", warehouse)

Review Comment:
   I'm not very familiar with the relationship between the warehouse and the 
catalog here, but based on my observations, some tests use the same client to 
create multiple catalogs while keeping the warehouse unchanged, so I think 
their names can be different.
   
   If the warehouse name is set to match the catalog name for this 
`IcebergHelper`, it may lead to a "warehouse not found" error. For example, in 
the test 
[testCatalogWithCustomMetricsReporter](https://github.com/apache/iceberg/blob/58b283e3533afce8b9884ac847a46eb40688353b/core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java#L2936),
 `initCatalog` is called with a catalog name `catalog_with_custom_reporter` 
that is different from default. This causes the test to search for a warehouse 
named `catalog_with_custom_reporter`, which doesn’t exist, as the warehouse was 
already initialized with a different name in `@BeforeEach` and didn't change.
   
   I'm not sure whether we could set warehouse name to a constant and we only 
change the catalog name in the tests



-- 
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-unsubscr...@polaris.apache.org

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

Reply via email to