dimas-b commented on code in PR #1068:
URL: https://github.com/apache/polaris/pull/1068#discussion_r1991545731
##########
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:
> To recap my understanding of the thread, there is no regression with this
change but it opens up the possibility for admins to raise the limit higher
than the default and then later lower it. If an admin decides to do this they
may see problems. Is that the case you're concerned about?
I agree that there's no regression when defaults are used.
If the admin sets lower values, this may cause previous storage config to
become inoperable, indeed.
My concern is that enforcing the new rules on config load (in the JSON
constructor) is an overkill in this situation. I do not see any benefit in this
enforcement and I do see drawbacks (as discussed).
I think it is perfectly valid to enforce the limit on config
creation/modification (basically at the API layer).
--
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]
