dennishuo commented on code in PR #1466:
URL: https://github.com/apache/polaris/pull/1466#discussion_r2074686001
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -234,4 +235,11 @@ public static void enforceFeatureEnabledOrThrow(
.description("If true, the policy-store endpoints are enabled")
.defaultValue(true)
.buildFeatureConfiguration();
+
+ public static final FeatureConfiguration<List<String>>
SUPPORTED_CATALOG_CONNECTION_TYPES =
+ PolarisConfiguration.<List<String>>builder()
+ .key("SUPPORTED_CATALOG_CONNECTION_TYPES")
+ .description("The list of supported catalog connection types for
federation")
+ .defaultValue(List.of(ConnectionType.ICEBERG_REST.name(),
ConnectionType.HADOOP.name()))
Review Comment:
Yes, we already have a feature gate for this:
["ENABLE_CATALOG_FEDERATION"](https://github.com/apache/polaris/blob/40aea3ac39ac485a76f3aaa930694eeab8ec14d8/quarkus/defaults/src/main/resources/application.properties#L113)
However, at the moment enabling that only risks "broken experimental
functionality" but not the security of the server, so the main concern would be
if any service providers happen to already be setting
`ENABLE_CATALOG_FEDERATION=true` but aren't paying attention to the addition of
new federation types.
Perhaps we can at least include the new `SUPPORTED_CATALOG_CONNECTION_TYPES`
in application.properties to explicitly list the two types, and maybe with a
commented-out version that only includes ICEBERG_REST as well, with some
comments about how the reader should choose the configuration. Or make the
default one only `[ICEBERG_REST]` with a commented-out version that includes
both `[ICEBERG_REST, HADOOP]`.
--
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
