Copilot commented on code in PR #13302:
URL: https://github.com/apache/gravitino/pull/13302#discussion_r4043817238
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConfig.java:
##########
@@ -379,14 +414,33 @@ public Map<String, String> getClientConfig() {
}
/**
- * Retrieves the single metalake mode.
+ * Whether Trino catalog names carry the metalake, as {@code
"<metalake>.<catalog>"}. The
+ * deprecated {@code gravitino.use-single-metalake=false} is honored when
the new key is unset.
*
- * @return the single metalake mode
+ * @return true if catalog names are qualified with the metalake
*/
- public boolean singleMetalakeMode() {
- return Boolean.parseBoolean(
- config.getOrDefault(
- GRAVITINO_SINGLE_METALAKE_MODE.key,
GRAVITINO_SINGLE_METALAKE_MODE.defaultValue));
+ public boolean catalogNameWithMetalake() {
+ String value = config.get(GRAVITINO_CATALOG_NAME_WITH_METALAKE.key);
+ if (value != null) {
+ return parseBooleanConfig(GRAVITINO_CATALOG_NAME_WITH_METALAKE.key,
value.trim());
+ }
+ return !parseBooleanConfig(
+ GRAVITINO_SINGLE_METALAKE_MODE.key,
+ config
+ .getOrDefault(
+ GRAVITINO_SINGLE_METALAKE_MODE.key,
GRAVITINO_SINGLE_METALAKE_MODE.defaultValue)
+ .trim());
+ }
Review Comment:
This change removes the public `singleMetalakeMode()` method from
`GravitinoConfig`, although the PR only deprecates the configuration key.
Consumers compiled against this public class will fail to compile or link;
retain a deprecated delegating alias that returns `!catalogNameWithMetalake()`
while migrating internal callers.
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConfig.java:
##########
@@ -815,11 +867,8 @@ public String getManualIcebergRestUri(String metalake) {
if (StringUtils.isNotBlank(scopedValue)) {
return scopedValue;
}
- if (singleMetalakeMode()) {
- return config.getOrDefault(
- GRAVITINO_ICEBERG_REST_URI.key,
GRAVITINO_ICEBERG_REST_URI.defaultValue);
- }
- return GRAVITINO_ICEBERG_REST_URI.defaultValue;
+ return config.getOrDefault(
+ GRAVITINO_ICEBERG_REST_URI.key,
GRAVITINO_ICEBERG_REST_URI.defaultValue);
Review Comment:
The new per-metalake URI override is not carried into dynamic catalog
configs: `toCatalogConfig()` serializes the exact `gravitino.iceberg.rest-uri`
entry but omits `gravitino.iceberg.rest-uri.<metalake>` keys. A worker
therefore cannot see this scoped override and falls back to the
unscoped/discovered endpoint, potentially routing the catalog to the wrong IRC.
Propagate scoped URI entries to the generated catalog configuration and cover
this through the registration/worker path.
--
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]
