gh-yzou commented on code in PR #1126:
URL: https://github.com/apache/polaris/pull/1126#discussion_r2004517639
##########
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 see. However, Polaris uses the warehouse as catalog name, it searches
warehouse among the all catalogs
https://github.com/apache/polaris/blob/main/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java#L605
In that sense, warehouse does mean catalog, where the original code seems
correct to me. Your following line of change seems correct to me.
For the point of
```
initCatalog is called with a catalog name catalog_with_custom_reporter that
is different from default
```
I think that is really a test setup problem, but i also see that you did a
change to overwrite the initCatalog with the correct setup of catalog by
creating catalog with the correct name, so your test should pass without the
change here, right? If that is the case, maybe we can avoid introducing
warehouse, which is kind of confusing about which parameter should be used, and
in general they should be the same.
--
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]
