poojanilangekar commented on code in PR #1959:
URL: https://github.com/apache/polaris/pull/1959#discussion_r2196069583


##########
polaris-core/src/main/java/org/apache/polaris/core/config/BehaviorChangeConfiguration.java:
##########
@@ -34,11 +35,12 @@ public class BehaviorChangeConfiguration<T> extends 
PolarisConfiguration<T> {
 
   protected BehaviorChangeConfiguration(
       String key,

Review Comment:
   I disagree with this:
   
   > It seems reasonable to expect that multiple different entries could be 
used to configure the same feature.
   
   You can have two configurations that store the same logical value and then 
let the specific module decide upon how to reconcile any conflicting values. 
But the config service should not register two configurations for the same 
feature. 
   
   Regarding the case where the `key` is `true` and `legacyKey` is false. I 
don't see how it is any different from a default value being defined say 
FeatureConfiguration.java, a value being defined in application.properties and 
then a JVM system property. In this case the system property takes precedence. 
Similarly the `key` takes precedence over `legacyKey`. 



-- 
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

Reply via email to