eric-maynard commented on code in PR #1068:
URL: https://github.com/apache/polaris/pull/1068#discussion_r1990598036
##########
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:
> In this context, my concern is not about how configuration works or how
configuration is described. My concern is with runtime behaviour in case the
new config settings make storage config validation fail.
I'm having troubling disentangling these two concepts. The unstable runtime
behavior you're describing only happens if an admin touches this flag that's
explicitly labeled as being a break-glass flag that may cause unstable
behavior. Twice.
> IMHO, because the storage config in question is present in the database.
It was stored some time ago under different rules.
The new rules are strictly higher than the old rules, so there is no
regression here.
> but again in that case, what is the value of failing to make unadjusted
storage config available for processing? It can cause hard failures compared to
hypothetical performance problems...
Can you say more about this? I don't quite follow the double-negatives here.
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. If an admin decides to do this and then later decides to
lower it again they may see problems. Is that the case you're concerned about?
--
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]