eric-maynard commented on code in PR #1068:
URL: https://github.com/apache/polaris/pull/1068#discussion_r1990193812


##########
polaris-core/src/main/java/org/apache/polaris/core/PolarisConfigurationStore.java:
##########
@@ -117,4 +122,16 @@ public interface PolarisConfigurationStore {
       return getConfiguration(ctx, config);
     }
   }
+
+  public static <T> @Nonnull T getConfiguration(PolarisConfiguration<T> 
configuration) {
+    CallContext callContext = CallContext.getCurrentContext();
+    if (callContext == null) {
+      return configuration.defaultValue;

Review Comment:
   When is it not valid to use the default value if CallContext it not 
available?
   
   CallContext is only ever via `CallContext.getCurrentContext()` during tests, 
but my thinking was that it's outside the scope of this PR to go and fix all 
those tests. If you remove the check on line 128 and run the tests, you can see 
a handful of tests fail when they try to create storage configurations. If 
you'd prefer, I can try to fix those tests here.
   
   It's not the case that this affects all config variables though, as no other 
config variable check currently uses this code path. Only the new one.



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

Reply via email to