rexminnis commented on code in PR #16007:
URL: https://github.com/apache/iceberg/pull/16007#discussion_r3193452441
##########
open-api/src/testFixtures/java/org/apache/iceberg/rest/RESTCatalogServer.java:
##########
@@ -98,6 +103,8 @@ private CatalogContext initializeBackendCatalog() throws
IOException {
LOG.info("No warehouse location set. Defaulting to temp location: {}",
warehouseLocation);
}
+ resolveCatalogNameFromEnv(catalogProperties, System.getenv());
Review Comment:
Hi @ebyhr — gentle nudge here. To recap my earlier reply:
RCKUtils.environmentCatalogConfig() transforms CATALOG_NAME into the property
key name (it strips the CATALOG_ prefix and lowercases what's left,
with no underscores remaining). The downstream
PropertyUtil.propertyAsString(catalogProperties, "catalog.name",
CATALOG_NAME_DEFAULT) lookup is for catalog.name, so the env var silently falls
back to the
rest_backend default. The change here just bridges that one mismatch by
lifting CATALOG_NAME env → catalog.name property before the lookup; everything
else (CATALOG_* → arbitrary catalog properties via the
existing transform) is untouched.
Does that resolve the question, or would you prefer a different approach
(e.g., teaching environmentCatalogConfig itself to special-case CATALOG_NAME)?
Happy to update either way. CI is green and the change is small (8 lines + a
test).
--
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]
