pavibhai commented on code in PR #1068:
URL: https://github.com/apache/polaris/pull/1068#discussion_r1974570113
##########
polaris-core/src/main/java/org/apache/polaris/core/PolarisConfiguration.java:
##########
@@ -264,4 +264,11 @@ public static <T> Builder<T> builder() {
+ STORAGE_CREDENTIAL_DURATION_SECONDS.key)
.defaultValue(30 * 60) // 30 minutes
.build();
+
+ public static final PolarisConfiguration<Integer>
STORAGE_CONFIGURATION_MAX_LOCATIONS =
+ PolarisConfiguration.<Integer>builder()
+ .key("STORAGE_CONFIGURATION_MAX_LOCATIONS")
+ .description("How many locations can be associated with a storage
configuration")
+ .defaultValue(20)
Review Comment:
> That's right. I would be okay with removing a limit totally, but on the
other hand @collado-mike is mentioning the possibility of having 3 limits. I
think preserving at least one limit, so we have the concept of a limit, may be
helpful in the future especially if we do push storage configs down to the
table level.
+1 to this removing the option or making it not mandatory. Given that the
policy content comes from the tables paths and not the catalog level paths. The
key question that comes up is regarding the purpose of the defining the allowed
locations at the catalog.
In terms of restrictions on what locations can be vended this is
accomplished say in the case of AWS through the IAM Role that is configured. So
in this case is the purpose of these paths to serve as documentation (on what
locations should be available to the IAM Role?).
--
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]
