eric-maynard commented on code in PR #1068:
URL: https://github.com/apache/polaris/pull/1068#discussion_r1990416941
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpStorageConfigurationInfo.java:
##########
@@ -44,7 +40,9 @@ public GcpStorageConfigurationInfo(
@JsonProperty(value = "allowedLocations", required = true) @Nonnull
List<String> allowedLocations) {
super(StorageType.GCS, allowedLocations);
- validateMaxAllowedLocations(MAX_ALLOWED_LOCATIONS);
+ validateMaxAllowedLocations(
+ PolarisConfigurationStore.getConfiguration(
+ PolarisConfiguration.STORAGE_CONFIGURATION_MAX_LOCATIONS));
Review Comment:
I agree that the behavior here with a limit set is far from ideal.
I think this will be more clear once we close the discussion on #1124, but
in essence this code should never be hit unless the user has touched a flag
that's described there as:
> ... These flags controlsubtle behavior adjustments and bug fixes, not
user-facing catalog settings. They are intended or internal use only, are
inherently unstable, ...
Going forward with these flags, I think we should consider that sometimes
the behavior you get with the flag set to a non-default value is a poor or even
broken experience. The only reason the problematic code (this entire check, in
this case) is left in place is due to some perceived risk or potential
regression associated with removing it entirely. Eventually, we will want to
remove it entirely. #1124 says of this:
> Flags here are generally short-lived and should either be removed or
promoted to stable feature flags before the next release.
--
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]
