rexminnis commented on code in PR #16007:
URL: https://github.com/apache/iceberg/pull/16007#discussion_r3098789177


##########
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:
   Thanks for the review, @ebyhr.
   
   `environmentCatalogConfig()` turns `CATALOG_NAME` into the property key 
`name`, not `catalog.name`:
   
   ```
   CATALOG_NAME
     → strip "CATALOG_" prefix  → "NAME"
     → "__" → "-"               → "NAME"
     → "_"  → "."               → "NAME"  (no underscores left)
     → lowerCase                → "name"
   ```
   
   But `RESTCatalogServer` reads property `catalog.name` (`CATALOG_NAME` 
constant on line 48 = `"catalog.name"`). So env `CATALOG_NAME=foo` ends up as 
property `name`, which is ignored, and the backend catalog stays `rest_backend`.
   
   To set `catalog.name` via the existing scheme, a user has to set 
`CATALOG_CATALOG_NAME=foo` — which #14972 calls out as unintuitive. This PR 
adds the shorter `CATALOG_NAME` env var that users naturally try first.
   
   Happy to rework if you'd prefer a different shape — e.g., just documenting 
`CATALOG_CATALOG_NAME` without a new code path — but the reporter explicitly 
asked for a simpler env var.



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

Reply via email to