You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "dimas-b (via GitHub)" <gi...@apache.org> on 2026/01/27 19:54:23 UTC

[PR] Introduce RealmConfigurationSource [polaris]

dimas-b opened a new pull request, #3573:
URL: https://github.com/apache/polaris/pull/3573

   The existing `PolarisConfigurationStore` interface is a fixture of SPI methods providing config values from the environment and utility methods for various lookup parameter permutations and type casts.
   
   This change adds `RealmConfigurationSource` for the SPI part and moves lookup logic into `RealmConfigImpl`.
   
   Note that existing service code always accesses config via `RealmConfig`.
   
   The old `PolarisConfigurationStore` interface remain for backward compatibility and but redirects actual config lookup to `RealmConfigImpl`.
   
   <!--
   ๐Ÿ“ Describe what changes you're proposing, especially breaking or user-facing changes. 
   ๐Ÿ“– See https://github.com/apache/polaris/blob/main/CONTRIBUTING.md for more.
   -->
   
   ## Checklist
   - [ ] ๐Ÿ›ก๏ธ Don't disclose security issues! (contact security@apache.org)
   - [ ] ๐Ÿ”— Clearly explained why the changes are needed, or linked related issues: Fixes #
   - [ ] ๐Ÿงช Added/updated tests with good coverage, or manually tested (and explained how)
   - [ ] ๐Ÿ’ก Added comments for complex logic
   - [ ] ๐Ÿงพ Updated `CHANGELOG.md` (if needed)
   - [ ] ๐Ÿ“š Updated documentation in `site/content/in-dev/unreleased` (if needed)
   


-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Introduce RealmConfigurationSource [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3573:
URL: https://github.com/apache/polaris/pull/3573#discussion_r2775039778


##########
polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java:
##########
@@ -225,18 +225,8 @@ public void testCacheMissForAnotherPrincipal() {
     Mockito.when(storageCredentialsVendor.getRealmConfig())
         .thenReturn(
             new RealmConfigImpl(
-                new PolarisConfigurationStore() {
-                  @SuppressWarnings("unchecked")
-                  @Override
-                  public String getConfiguration(@Nonnull RealmContext ctx, String configName) {
-                    if (configName.equals(
-                        FeatureConfiguration.INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL
-                            .key())) {
-                      return "true";
-                    }
-                    return null;
-                  }
-                },
+                (rc, name) ->
+                    Map.of(INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL.key(), "true").get(name),

Review Comment:
   refactored



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Introduce RealmConfigurationSource [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b merged PR #3573:
URL: https://github.com/apache/polaris/pull/3573


-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Introduce RealmConfigurationSource [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3573:
URL: https://github.com/apache/polaris/pull/3573#discussion_r2774882213


##########
polaris-core/src/main/java/org/apache/polaris/core/PolarisCallContext.java:
##########
@@ -34,23 +34,36 @@ public class PolarisCallContext implements CallContext {
 
   // meta store which is used to persist Polaris entity metadata
   private final BasePersistence metaStore;
-  private final PolarisConfigurationStore configurationStore;
+  private final RealmConfigurationSource configurationSource;
   private final RealmContext realmContext;
   private final RealmConfig realmConfig;
 
+  /**
+   * @deprecated Use {@link PolarisCallContext#PolarisCallContext(RealmContext, BasePersistence,
+   *     RealmConfigurationSource)}.
+   */
+  @SuppressWarnings("removal")
+  @Deprecated(forRemoval = true)
   public PolarisCallContext(
       @Nonnull RealmContext realmContext,
       @Nonnull BasePersistence metaStore,
-      @Nonnull PolarisConfigurationStore configurationStore) {
+      @Nonnull org.apache.polaris.core.config.PolarisConfigurationStore configurationStore) {

Review Comment:
   The warning is specifically about "deprecated for removal" classes. Ordinary deprecation is not flagged on import statements.



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Introduce RealmConfigurationSource [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3573:
URL: https://github.com/apache/polaris/pull/3573#discussion_r2765231485


##########
polaris-core/src/main/java/org/apache/polaris/core/config/RealmConfigImpl.java:
##########
@@ -18,43 +18,105 @@
  */
 package org.apache.polaris.core.config;
 
+import jakarta.annotation.Nonnull;
 import jakarta.annotation.Nullable;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 import org.apache.polaris.core.context.RealmContext;
 import org.apache.polaris.core.entity.CatalogEntity;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class RealmConfigImpl implements RealmConfig {
+  private static final Logger LOGGER = LoggerFactory.getLogger(RealmConfigImpl.class);
 
-  private final PolarisConfigurationStore configurationStore;
+  private final RealmConfigurationSource configurationSource;
   private final RealmContext realmContext;
 
-  public RealmConfigImpl(PolarisConfigurationStore configurationStore, RealmContext realmContext) {
-    this.configurationStore = configurationStore;
+  public RealmConfigImpl(RealmConfigurationSource configurationSource, RealmContext realmContext) {
+    this.configurationSource = configurationSource;
     this.realmContext = realmContext;
   }
 
+  @SuppressWarnings("removal")
   @Override
   public <T> @Nullable T getConfig(String configName) {
-    return configurationStore.getConfiguration(realmContext, configName);
+    @SuppressWarnings("unchecked")
+    T value = (T) configurationSource.getConfigValue(realmContext, configName);
+    return value;
   }
 
+  @SuppressWarnings("removal")
   @Override
   public <T> T getConfig(String configName, T defaultValue) {
-    return configurationStore.getConfiguration(realmContext, configName, defaultValue);
+    @SuppressWarnings("unchecked")
+    T value = (T) getConfig(configName);
+    if (value == null) {
+      return defaultValue;
+    }
+    return value;
   }
 
   @Override
   public <T> T getConfig(PolarisConfiguration<T> config) {
-    return configurationStore.getConfiguration(realmContext, config);
+    return getConfig(config, Collections.emptyMap());
   }
 
   @Override
   public <T> T getConfig(PolarisConfiguration<T> config, CatalogEntity catalogEntity) {
-    return configurationStore.getConfiguration(realmContext, catalogEntity, config);
+    return getConfig(config, catalogEntity.getPropertiesAsMap());
   }
 
   @Override
   public <T> T getConfig(PolarisConfiguration<T> config, Map<String, String> catalogProperties) {
-    return configurationStore.getConfiguration(realmContext, catalogProperties, config);
+    Object propertyValue = null;
+    if (config.hasCatalogConfig() || config.hasCatalogConfigUnsafe()) {
+      if (config.hasCatalogConfig()) {
+        propertyValue = catalogProperties.get(config.catalogConfig());
+      }
+      if (propertyValue == null) {
+        if (config.hasCatalogConfigUnsafe()) {
+          propertyValue = catalogProperties.get(config.catalogConfigUnsafe());
+        }
+        if (propertyValue != null) {
+          LOGGER.warn(
+              String.format(
+                  "Deprecated config %s is in use and will be removed in a future version",
+                  config.catalogConfigUnsafe()));
+        }
+      }
+    }
+
+    if (propertyValue == null) {
+      propertyValue = configurationSource.getConfigValue(realmContext, config.key());
+    }
+
+    return tryCast(config, propertyValue);
+  }
+
+  /**
+   * In some cases, we may extract a value that doesn't match the expected type for a config. This
+   * method can be used to attempt to force-cast it using `String.valueOf`
+   */
+  private <T> @Nonnull T tryCast(PolarisConfiguration<T> config, Object value) {
+    if (value == null) {
+      return config.defaultValue();
+    }
+
+    if (config.defaultValue() instanceof Boolean) {
+      return config.cast(Boolean.valueOf(String.valueOf(value)));
+    } else if (config.defaultValue() instanceof Integer) {
+      return config.cast(Integer.valueOf(String.valueOf(value)));
+    } else if (config.defaultValue() instanceof Long) {
+      return config.cast(Long.valueOf(String.valueOf(value)));
+    } else if (config.defaultValue() instanceof Double) {
+      return config.cast(Double.valueOf(String.valueOf(value)));
+    } else if (config.defaultValue() instanceof List<?>) {
+      return config.cast(new ArrayList<>((List<?>) value));

Review Comment:
   Probably, but I did not want to mix pure refactoring with logic changes :sweat_smile: 



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Introduce RealmConfigurationSource [polaris]

Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on code in PR #3573:
URL: https://github.com/apache/polaris/pull/3573#discussion_r2772628625


##########
polaris-core/src/main/java/org/apache/polaris/core/config/RealmConfigImpl.java:
##########
@@ -18,43 +18,105 @@
  */
 package org.apache.polaris.core.config;
 
+import jakarta.annotation.Nonnull;
 import jakarta.annotation.Nullable;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 import org.apache.polaris.core.context.RealmContext;
 import org.apache.polaris.core.entity.CatalogEntity;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class RealmConfigImpl implements RealmConfig {
+  private static final Logger LOGGER = LoggerFactory.getLogger(RealmConfigImpl.class);
 
-  private final PolarisConfigurationStore configurationStore;
+  private final RealmConfigurationSource configurationSource;
   private final RealmContext realmContext;
 
-  public RealmConfigImpl(PolarisConfigurationStore configurationStore, RealmContext realmContext) {
-    this.configurationStore = configurationStore;
+  public RealmConfigImpl(RealmConfigurationSource configurationSource, RealmContext realmContext) {
+    this.configurationSource = configurationSource;
     this.realmContext = realmContext;
   }
 
+  @SuppressWarnings("removal")
   @Override
   public <T> @Nullable T getConfig(String configName) {
-    return configurationStore.getConfiguration(realmContext, configName);
+    @SuppressWarnings("unchecked")
+    T value = (T) configurationSource.getConfigValue(realmContext, configName);
+    return value;
   }
 
+  @SuppressWarnings("removal")
   @Override
   public <T> T getConfig(String configName, T defaultValue) {
-    return configurationStore.getConfiguration(realmContext, configName, defaultValue);
+    @SuppressWarnings("unchecked")
+    T value = (T) getConfig(configName);
+    if (value == null) {
+      return defaultValue;
+    }
+    return value;
   }
 
   @Override
   public <T> T getConfig(PolarisConfiguration<T> config) {
-    return configurationStore.getConfiguration(realmContext, config);
+    return getConfig(config, Collections.emptyMap());
   }
 
   @Override
   public <T> T getConfig(PolarisConfiguration<T> config, CatalogEntity catalogEntity) {
-    return configurationStore.getConfiguration(realmContext, catalogEntity, config);
+    return getConfig(config, catalogEntity.getPropertiesAsMap());
   }
 
   @Override
   public <T> T getConfig(PolarisConfiguration<T> config, Map<String, String> catalogProperties) {
-    return configurationStore.getConfiguration(realmContext, catalogProperties, config);
+    Object propertyValue = null;
+    if (config.hasCatalogConfig() || config.hasCatalogConfigUnsafe()) {
+      if (config.hasCatalogConfig()) {
+        propertyValue = catalogProperties.get(config.catalogConfig());
+      }
+      if (propertyValue == null) {
+        if (config.hasCatalogConfigUnsafe()) {
+          propertyValue = catalogProperties.get(config.catalogConfigUnsafe());
+        }
+        if (propertyValue != null) {
+          LOGGER.warn(
+              String.format(
+                  "Deprecated config %s is in use and will be removed in a future version",
+                  config.catalogConfigUnsafe()));
+        }
+      }
+    }
+
+    if (propertyValue == null) {
+      propertyValue = configurationSource.getConfigValue(realmContext, config.key());
+    }
+
+    return tryCast(config, propertyValue);
+  }
+
+  /**
+   * In some cases, we may extract a value that doesn't match the expected type for a config. This
+   * method can be used to attempt to force-cast it using `String.valueOf`
+   */
+  private <T> @Nonnull T tryCast(PolarisConfiguration<T> config, Object value) {
+    if (value == null) {
+      return config.defaultValue();
+    }
+
+    if (config.defaultValue() instanceof Boolean) {
+      return config.cast(Boolean.valueOf(String.valueOf(value)));

Review Comment:
   Yea - but I think it needs to be tackled in a follow-up.
   Configurations are often used on hot code path.
   Same for below.



##########
polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java:
##########
@@ -225,18 +225,8 @@ public void testCacheMissForAnotherPrincipal() {
     Mockito.when(storageCredentialsVendor.getRealmConfig())
         .thenReturn(
             new RealmConfigImpl(
-                new PolarisConfigurationStore() {
-                  @SuppressWarnings("unchecked")
-                  @Override
-                  public String getConfiguration(@Nonnull RealmContext ctx, String configName) {
-                    if (configName.equals(
-                        FeatureConfiguration.INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL
-                            .key())) {
-                      return "true";
-                    }
-                    return null;
-                  }
-                },
+                (rc, name) ->
+                    Map.of(INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL.key(), "true").get(name),

Review Comment:
   Maybe, but Alex' way is clearer.
   
   (Troll: Without a formatter, we could squeeze the whole file into a single line :P )



##########
runtime/service/src/main/java/org/apache/polaris/service/config/PolarisConfigurationStoreBridge.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.service.config;
+
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.inject.Inject;
+import org.apache.polaris.core.context.RealmContext;
+
+@SuppressWarnings("removal")
+@ApplicationScoped
+public class PolarisConfigurationStoreBridge

Review Comment:
   What's the use case of this application-scoped bean?
   I only see one usage in a test. Maybe refactor the test or move this type to `testFixtures`.
   



##########
polaris-core/src/main/java/org/apache/polaris/core/PolarisCallContext.java:
##########
@@ -34,23 +34,36 @@ public class PolarisCallContext implements CallContext {
 
   // meta store which is used to persist Polaris entity metadata
   private final BasePersistence metaStore;
-  private final PolarisConfigurationStore configurationStore;
+  private final RealmConfigurationSource configurationSource;
   private final RealmContext realmContext;
   private final RealmConfig realmConfig;
 
+  /**
+   * @deprecated Use {@link PolarisCallContext#PolarisCallContext(RealmContext, BasePersistence,
+   *     RealmConfigurationSource)}.
+   */
+  @SuppressWarnings("removal")
+  @Deprecated(forRemoval = true)
   public PolarisCallContext(
       @Nonnull RealmContext realmContext,
       @Nonnull BasePersistence metaStore,
-      @Nonnull PolarisConfigurationStore configurationStore) {
+      @Nonnull org.apache.polaris.core.config.PolarisConfigurationStore configurationStore) {

Review Comment:
   > IntelliJ warns about deprecation on the import statement
   
   This is strange, because that Java 8 legacy was removed via [JEP 211](https://openjdk.org/jeps/211).



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Introduce RealmConfigurationSource [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3573:
URL: https://github.com/apache/polaris/pull/3573#discussion_r2765234734


##########
polaris-core/src/main/java/org/apache/polaris/core/config/RealmConfigImpl.java:
##########
@@ -18,43 +18,105 @@
  */
 package org.apache.polaris.core.config;
 
+import jakarta.annotation.Nonnull;
 import jakarta.annotation.Nullable;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 import org.apache.polaris.core.context.RealmContext;
 import org.apache.polaris.core.entity.CatalogEntity;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class RealmConfigImpl implements RealmConfig {
+  private static final Logger LOGGER = LoggerFactory.getLogger(RealmConfigImpl.class);
 
-  private final PolarisConfigurationStore configurationStore;
+  private final RealmConfigurationSource configurationSource;
   private final RealmContext realmContext;
 
-  public RealmConfigImpl(PolarisConfigurationStore configurationStore, RealmContext realmContext) {
-    this.configurationStore = configurationStore;
+  public RealmConfigImpl(RealmConfigurationSource configurationSource, RealmContext realmContext) {
+    this.configurationSource = configurationSource;
     this.realmContext = realmContext;
   }
 
+  @SuppressWarnings("removal")
   @Override
   public <T> @Nullable T getConfig(String configName) {
-    return configurationStore.getConfiguration(realmContext, configName);
+    @SuppressWarnings("unchecked")
+    T value = (T) configurationSource.getConfigValue(realmContext, configName);
+    return value;
   }
 
+  @SuppressWarnings("removal")
   @Override
   public <T> T getConfig(String configName, T defaultValue) {
-    return configurationStore.getConfiguration(realmContext, configName, defaultValue);
+    @SuppressWarnings("unchecked")
+    T value = (T) getConfig(configName);
+    if (value == null) {
+      return defaultValue;
+    }
+    return value;
   }
 
   @Override
   public <T> T getConfig(PolarisConfiguration<T> config) {
-    return configurationStore.getConfiguration(realmContext, config);
+    return getConfig(config, Collections.emptyMap());
   }
 
   @Override
   public <T> T getConfig(PolarisConfiguration<T> config, CatalogEntity catalogEntity) {
-    return configurationStore.getConfiguration(realmContext, catalogEntity, config);
+    return getConfig(config, catalogEntity.getPropertiesAsMap());
   }
 
   @Override
   public <T> T getConfig(PolarisConfiguration<T> config, Map<String, String> catalogProperties) {
-    return configurationStore.getConfiguration(realmContext, catalogProperties, config);
+    Object propertyValue = null;
+    if (config.hasCatalogConfig() || config.hasCatalogConfigUnsafe()) {
+      if (config.hasCatalogConfig()) {
+        propertyValue = catalogProperties.get(config.catalogConfig());
+      }
+      if (propertyValue == null) {
+        if (config.hasCatalogConfigUnsafe()) {
+          propertyValue = catalogProperties.get(config.catalogConfigUnsafe());
+        }
+        if (propertyValue != null) {
+          LOGGER.warn(
+              String.format(
+                  "Deprecated config %s is in use and will be removed in a future version",
+                  config.catalogConfigUnsafe()));
+        }
+      }
+    }
+
+    if (propertyValue == null) {
+      propertyValue = configurationSource.getConfigValue(realmContext, config.key());
+    }
+
+    return tryCast(config, propertyValue);
+  }
+
+  /**
+   * In some cases, we may extract a value that doesn't match the expected type for a config. This
+   * method can be used to attempt to force-cast it using `String.valueOf`
+   */
+  private <T> @Nonnull T tryCast(PolarisConfiguration<T> config, Object value) {
+    if (value == null) {
+      return config.defaultValue();
+    }
+
+    if (config.defaultValue() instanceof Boolean) {
+      return config.cast(Boolean.valueOf(String.valueOf(value)));

Review Comment:
   I did not want to mix pure refactoring with logic changes :sweat_smile: This code was moved "as is".



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Introduce RealmConfigurationSource [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3573:
URL: https://github.com/apache/polaris/pull/3573#discussion_r2770720876


##########
polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfigurationStore.java:
##########
@@ -57,36 +54,13 @@ public interface PolarisConfigurationStore {
    * @param defaultValue the default value if the configuration key has no value
    * @return the current value or the supplied default value
    * @param <T> the type of the configuration value
+   * @deprecated Use {@link RealmConfig}.
    */
+  @SuppressWarnings({"DeprecatedIsStillUsed", "removal"})
+  @Deprecated(forRemoval = true)

Review Comment:
   updated



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Introduce RealmConfigurationSource [polaris]

Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #3573:
URL: https://github.com/apache/polaris/pull/3573#discussion_r2770098103


##########
polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfigurationStore.java:
##########
@@ -57,36 +54,13 @@ public interface PolarisConfigurationStore {
    * @param defaultValue the default value if the configuration key has no value
    * @return the current value or the supplied default value
    * @param <T> the type of the configuration value
+   * @deprecated Use {@link RealmConfig}.
    */
+  @SuppressWarnings({"DeprecatedIsStillUsed", "removal"})
+  @Deprecated(forRemoval = true)

Review Comment:
   Should the deprecation target the whole class?



##########
polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java:
##########
@@ -225,18 +225,8 @@ public void testCacheMissForAnotherPrincipal() {
     Mockito.when(storageCredentialsVendor.getRealmConfig())
         .thenReturn(
             new RealmConfigImpl(
-                new PolarisConfigurationStore() {
-                  @SuppressWarnings("unchecked")
-                  @Override
-                  public String getConfiguration(@Nonnull RealmContext ctx, String configName) {
-                    if (configName.equals(
-                        FeatureConfiguration.INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL
-                            .key())) {
-                      return "true";
-                    }
-                    return null;
-                  }
-                },
+                (rc, name) ->
+                    Map.of(INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL.key(), "true").get(name),

Review Comment:
   nit: isn't this simpler?
   
   ```suggestion
                       INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL.key().equals(name) ? "true" : null,
   ```
   
   I've seen other similar occurrences in this PR.



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Introduce RealmConfigurationSource [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3573:
URL: https://github.com/apache/polaris/pull/3573#discussion_r2770599912


##########
polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java:
##########
@@ -225,18 +225,8 @@ public void testCacheMissForAnotherPrincipal() {
     Mockito.when(storageCredentialsVendor.getRealmConfig())
         .thenReturn(
             new RealmConfigImpl(
-                new PolarisConfigurationStore() {
-                  @SuppressWarnings("unchecked")
-                  @Override
-                  public String getConfiguration(@Nonnull RealmContext ctx, String configName) {
-                    if (configName.equals(
-                        FeatureConfiguration.INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL
-                            .key())) {
-                      return "true";
-                    }
-                    return null;
-                  }
-                },
+                (rc, name) ->
+                    Map.of(INCLUDE_PRINCIPAL_NAME_IN_SUBSCOPED_CREDENTIAL.key(), "true").get(name),

Review Comment:
   With our formatter, current code is 1 line, the proposed code will take 3 lines :sweat_smile: 



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Introduce RealmConfigurationSource [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3573:
URL: https://github.com/apache/polaris/pull/3573#discussion_r2774897948


##########
runtime/service/src/main/java/org/apache/polaris/service/config/PolarisConfigurationStoreBridge.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.service.config;
+
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.inject.Inject;
+import org.apache.polaris.core.context.RealmContext;
+
+@SuppressWarnings("removal")
+@ApplicationScoped
+public class PolarisConfigurationStoreBridge

Review Comment:
   Supporting legacy code that might depend on `PolarisConfigurationStore` being injected by CDI.



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Introduce RealmConfigurationSource [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3573:
URL: https://github.com/apache/polaris/pull/3573#discussion_r2774941779


##########
runtime/service/src/main/java/org/apache/polaris/service/config/PolarisConfigurationStoreBridge.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.service.config;
+
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.inject.Inject;
+import org.apache.polaris.core.context.RealmContext;
+
+@SuppressWarnings("removal")
+@ApplicationScoped
+public class PolarisConfigurationStoreBridge

Review Comment:
   removed this class - `DefaultConfigurationStore` implements `PolarisConfigurationStore` as before.



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Introduce RealmConfigurationSource [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3573:
URL: https://github.com/apache/polaris/pull/3573#discussion_r2770732689


##########
polaris-core/src/main/java/org/apache/polaris/core/PolarisCallContext.java:
##########
@@ -34,23 +34,36 @@ public class PolarisCallContext implements CallContext {
 
   // meta store which is used to persist Polaris entity metadata
   private final BasePersistence metaStore;
-  private final PolarisConfigurationStore configurationStore;
+  private final RealmConfigurationSource configurationSource;
   private final RealmContext realmContext;
   private final RealmConfig realmConfig;
 
+  /**
+   * @deprecated Use {@link PolarisCallContext#PolarisCallContext(RealmContext, BasePersistence,
+   *     RealmConfigurationSource)}.
+   */
+  @SuppressWarnings("removal")
+  @Deprecated(forRemoval = true)
   public PolarisCallContext(
       @Nonnull RealmContext realmContext,
       @Nonnull BasePersistence metaStore,
-      @Nonnull PolarisConfigurationStore configurationStore) {
+      @Nonnull org.apache.polaris.core.config.PolarisConfigurationStore configurationStore) {

Review Comment:
   note: avoiding imports because IntelliJ warns about deprecation on the import statement and that warning cannot be suppressed ๐Ÿคท 



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Introduce RealmConfigurationSource [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3573:
URL: https://github.com/apache/polaris/pull/3573#discussion_r2774894010


##########
polaris-core/src/main/java/org/apache/polaris/core/config/RealmConfigImpl.java:
##########
@@ -18,43 +18,105 @@
  */
 package org.apache.polaris.core.config;
 
+import jakarta.annotation.Nonnull;
 import jakarta.annotation.Nullable;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 import org.apache.polaris.core.context.RealmContext;
 import org.apache.polaris.core.entity.CatalogEntity;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class RealmConfigImpl implements RealmConfig {
+  private static final Logger LOGGER = LoggerFactory.getLogger(RealmConfigImpl.class);
 
-  private final PolarisConfigurationStore configurationStore;
+  private final RealmConfigurationSource configurationSource;
   private final RealmContext realmContext;
 
-  public RealmConfigImpl(PolarisConfigurationStore configurationStore, RealmContext realmContext) {
-    this.configurationStore = configurationStore;
+  public RealmConfigImpl(RealmConfigurationSource configurationSource, RealmContext realmContext) {
+    this.configurationSource = configurationSource;
     this.realmContext = realmContext;
   }
 
+  @SuppressWarnings("removal")
   @Override
   public <T> @Nullable T getConfig(String configName) {
-    return configurationStore.getConfiguration(realmContext, configName);
+    @SuppressWarnings("unchecked")
+    T value = (T) configurationSource.getConfigValue(realmContext, configName);
+    return value;
   }
 
+  @SuppressWarnings("removal")
   @Override
   public <T> T getConfig(String configName, T defaultValue) {
-    return configurationStore.getConfiguration(realmContext, configName, defaultValue);
+    @SuppressWarnings("unchecked")
+    T value = (T) getConfig(configName);
+    if (value == null) {
+      return defaultValue;
+    }
+    return value;
   }
 
   @Override
   public <T> T getConfig(PolarisConfiguration<T> config) {
-    return configurationStore.getConfiguration(realmContext, config);
+    return getConfig(config, Collections.emptyMap());
   }
 
   @Override
   public <T> T getConfig(PolarisConfiguration<T> config, CatalogEntity catalogEntity) {
-    return configurationStore.getConfiguration(realmContext, catalogEntity, config);
+    return getConfig(config, catalogEntity.getPropertiesAsMap());
   }
 
   @Override
   public <T> T getConfig(PolarisConfiguration<T> config, Map<String, String> catalogProperties) {
-    return configurationStore.getConfiguration(realmContext, catalogProperties, config);
+    Object propertyValue = null;
+    if (config.hasCatalogConfig() || config.hasCatalogConfigUnsafe()) {
+      if (config.hasCatalogConfig()) {
+        propertyValue = catalogProperties.get(config.catalogConfig());
+      }
+      if (propertyValue == null) {
+        if (config.hasCatalogConfigUnsafe()) {
+          propertyValue = catalogProperties.get(config.catalogConfigUnsafe());
+        }
+        if (propertyValue != null) {
+          LOGGER.warn(
+              String.format(
+                  "Deprecated config %s is in use and will be removed in a future version",
+                  config.catalogConfigUnsafe()));
+        }
+      }
+    }
+
+    if (propertyValue == null) {
+      propertyValue = configurationSource.getConfigValue(realmContext, config.key());
+    }
+
+    return tryCast(config, propertyValue);
+  }
+
+  /**
+   * In some cases, we may extract a value that doesn't match the expected type for a config. This
+   * method can be used to attempt to force-cast it using `String.valueOf`
+   */
+  private <T> @Nonnull T tryCast(PolarisConfiguration<T> config, Object value) {
+    if (value == null) {
+      return config.defaultValue();
+    }
+
+    if (config.defaultValue() instanceof Boolean) {
+      return config.cast(Boolean.valueOf(String.valueOf(value)));

Review Comment:
   #3688



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Introduce RealmConfigurationSource [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3573:
URL: https://github.com/apache/polaris/pull/3573#discussion_r2770578216


##########
polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfigurationStore.java:
##########
@@ -57,36 +54,13 @@ public interface PolarisConfigurationStore {
    * @param defaultValue the default value if the configuration key has no value
    * @return the current value or the supplied default value
    * @param <T> the type of the configuration value
+   * @deprecated Use {@link RealmConfig}.
    */
+  @SuppressWarnings({"DeprecatedIsStillUsed", "removal"})
+  @Deprecated(forRemoval = true)

Review Comment:
   Good point :+1: 



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Introduce RealmConfigurationSource [polaris]

Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on code in PR #3573:
URL: https://github.com/apache/polaris/pull/3573#discussion_r2764052463


##########
polaris-core/src/main/java/org/apache/polaris/core/config/RealmConfigImpl.java:
##########
@@ -18,43 +18,105 @@
  */
 package org.apache.polaris.core.config;
 
+import jakarta.annotation.Nonnull;
 import jakarta.annotation.Nullable;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 import org.apache.polaris.core.context.RealmContext;
 import org.apache.polaris.core.entity.CatalogEntity;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class RealmConfigImpl implements RealmConfig {
+  private static final Logger LOGGER = LoggerFactory.getLogger(RealmConfigImpl.class);
 
-  private final PolarisConfigurationStore configurationStore;
+  private final RealmConfigurationSource configurationSource;
   private final RealmContext realmContext;
 
-  public RealmConfigImpl(PolarisConfigurationStore configurationStore, RealmContext realmContext) {
-    this.configurationStore = configurationStore;
+  public RealmConfigImpl(RealmConfigurationSource configurationSource, RealmContext realmContext) {
+    this.configurationSource = configurationSource;
     this.realmContext = realmContext;
   }
 
+  @SuppressWarnings("removal")
   @Override
   public <T> @Nullable T getConfig(String configName) {
-    return configurationStore.getConfiguration(realmContext, configName);
+    @SuppressWarnings("unchecked")
+    T value = (T) configurationSource.getConfigValue(realmContext, configName);
+    return value;
   }
 
+  @SuppressWarnings("removal")
   @Override
   public <T> T getConfig(String configName, T defaultValue) {
-    return configurationStore.getConfiguration(realmContext, configName, defaultValue);
+    @SuppressWarnings("unchecked")
+    T value = (T) getConfig(configName);
+    if (value == null) {
+      return defaultValue;
+    }
+    return value;
   }
 
   @Override
   public <T> T getConfig(PolarisConfiguration<T> config) {
-    return configurationStore.getConfiguration(realmContext, config);
+    return getConfig(config, Collections.emptyMap());
   }
 
   @Override
   public <T> T getConfig(PolarisConfiguration<T> config, CatalogEntity catalogEntity) {
-    return configurationStore.getConfiguration(realmContext, catalogEntity, config);
+    return getConfig(config, catalogEntity.getPropertiesAsMap());
   }
 
   @Override
   public <T> T getConfig(PolarisConfiguration<T> config, Map<String, String> catalogProperties) {
-    return configurationStore.getConfiguration(realmContext, catalogProperties, config);
+    Object propertyValue = null;
+    if (config.hasCatalogConfig() || config.hasCatalogConfigUnsafe()) {
+      if (config.hasCatalogConfig()) {
+        propertyValue = catalogProperties.get(config.catalogConfig());
+      }
+      if (propertyValue == null) {
+        if (config.hasCatalogConfigUnsafe()) {
+          propertyValue = catalogProperties.get(config.catalogConfigUnsafe());
+        }
+        if (propertyValue != null) {
+          LOGGER.warn(
+              String.format(
+                  "Deprecated config %s is in use and will be removed in a future version",
+                  config.catalogConfigUnsafe()));
+        }
+      }
+    }
+
+    if (propertyValue == null) {
+      propertyValue = configurationSource.getConfigValue(realmContext, config.key());
+    }
+
+    return tryCast(config, propertyValue);
+  }
+
+  /**
+   * In some cases, we may extract a value that doesn't match the expected type for a config. This
+   * method can be used to attempt to force-cast it using `String.valueOf`
+   */
+  private <T> @Nonnull T tryCast(PolarisConfiguration<T> config, Object value) {
+    if (value == null) {
+      return config.defaultValue();
+    }
+
+    if (config.defaultValue() instanceof Boolean) {
+      return config.cast(Boolean.valueOf(String.valueOf(value)));

Review Comment:
   Not new to this PR:
   Looks like this function converts every value to a string and then parses that string even if the type matches, which feels overly expensive.



##########
polaris-core/src/main/java/org/apache/polaris/core/config/RealmConfigurationSource.java:
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.config;
+
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.Map;
+import org.apache.polaris.core.context.RealmContext;
+
+/**
+ * This is an SPI interface used by Polaris Core for loading base configuration values from the
+ * environment.
+ *
+ * <p>Core classes are expected to use {@link RealmConfig} for configuration lookup in runtime.
+ */
+public interface RealmConfigurationSource {
+  RealmConfigurationSource EMPTY_CONFIG = (rc, name) -> null;
+
+  static RealmConfigurationSource global(Map<String, ?> config) {

Review Comment:
   ```suggestion
     @VisibleForTesting
     static RealmConfigurationSource global(Map<String, ?> config) {
   ```
   ?



##########
polaris-core/src/main/java/org/apache/polaris/core/config/RealmConfigImpl.java:
##########
@@ -18,43 +18,105 @@
  */
 package org.apache.polaris.core.config;
 
+import jakarta.annotation.Nonnull;
 import jakarta.annotation.Nullable;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 import org.apache.polaris.core.context.RealmContext;
 import org.apache.polaris.core.entity.CatalogEntity;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class RealmConfigImpl implements RealmConfig {
+  private static final Logger LOGGER = LoggerFactory.getLogger(RealmConfigImpl.class);
 
-  private final PolarisConfigurationStore configurationStore;
+  private final RealmConfigurationSource configurationSource;
   private final RealmContext realmContext;
 
-  public RealmConfigImpl(PolarisConfigurationStore configurationStore, RealmContext realmContext) {
-    this.configurationStore = configurationStore;
+  public RealmConfigImpl(RealmConfigurationSource configurationSource, RealmContext realmContext) {
+    this.configurationSource = configurationSource;
     this.realmContext = realmContext;
   }
 
+  @SuppressWarnings("removal")
   @Override
   public <T> @Nullable T getConfig(String configName) {
-    return configurationStore.getConfiguration(realmContext, configName);
+    @SuppressWarnings("unchecked")
+    T value = (T) configurationSource.getConfigValue(realmContext, configName);
+    return value;
   }
 
+  @SuppressWarnings("removal")
   @Override
   public <T> T getConfig(String configName, T defaultValue) {
-    return configurationStore.getConfiguration(realmContext, configName, defaultValue);
+    @SuppressWarnings("unchecked")
+    T value = (T) getConfig(configName);
+    if (value == null) {
+      return defaultValue;
+    }
+    return value;
   }
 
   @Override
   public <T> T getConfig(PolarisConfiguration<T> config) {
-    return configurationStore.getConfiguration(realmContext, config);
+    return getConfig(config, Collections.emptyMap());
   }
 
   @Override
   public <T> T getConfig(PolarisConfiguration<T> config, CatalogEntity catalogEntity) {
-    return configurationStore.getConfiguration(realmContext, catalogEntity, config);
+    return getConfig(config, catalogEntity.getPropertiesAsMap());
   }
 
   @Override
   public <T> T getConfig(PolarisConfiguration<T> config, Map<String, String> catalogProperties) {
-    return configurationStore.getConfiguration(realmContext, catalogProperties, config);
+    Object propertyValue = null;
+    if (config.hasCatalogConfig() || config.hasCatalogConfigUnsafe()) {
+      if (config.hasCatalogConfig()) {
+        propertyValue = catalogProperties.get(config.catalogConfig());
+      }
+      if (propertyValue == null) {
+        if (config.hasCatalogConfigUnsafe()) {
+          propertyValue = catalogProperties.get(config.catalogConfigUnsafe());
+        }
+        if (propertyValue != null) {
+          LOGGER.warn(
+              String.format(
+                  "Deprecated config %s is in use and will be removed in a future version",
+                  config.catalogConfigUnsafe()));
+        }
+      }
+    }
+
+    if (propertyValue == null) {
+      propertyValue = configurationSource.getConfigValue(realmContext, config.key());
+    }
+
+    return tryCast(config, propertyValue);
+  }
+
+  /**
+   * In some cases, we may extract a value that doesn't match the expected type for a config. This
+   * method can be used to attempt to force-cast it using `String.valueOf`
+   */
+  private <T> @Nonnull T tryCast(PolarisConfiguration<T> config, Object value) {
+    if (value == null) {
+      return config.defaultValue();
+    }
+
+    if (config.defaultValue() instanceof Boolean) {
+      return config.cast(Boolean.valueOf(String.valueOf(value)));
+    } else if (config.defaultValue() instanceof Integer) {
+      return config.cast(Integer.valueOf(String.valueOf(value)));
+    } else if (config.defaultValue() instanceof Long) {
+      return config.cast(Long.valueOf(String.valueOf(value)));
+    } else if (config.defaultValue() instanceof Double) {
+      return config.cast(Double.valueOf(String.valueOf(value)));
+    } else if (config.defaultValue() instanceof List<?>) {
+      return config.cast(new ArrayList<>((List<?>) value));

Review Comment:
   Not new to this PR, but shouldn't this yield an immutable/non-modifiable list instead?



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Introduce RealmConfigurationSource [polaris]

Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3573:
URL: https://github.com/apache/polaris/pull/3573#discussion_r2765290450


##########
polaris-core/src/main/java/org/apache/polaris/core/config/RealmConfigurationSource.java:
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.polaris.core.config;
+
+import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
+import java.util.Map;
+import org.apache.polaris.core.context.RealmContext;
+
+/**
+ * This is an SPI interface used by Polaris Core for loading base configuration values from the
+ * environment.
+ *
+ * <p>Core classes are expected to use {@link RealmConfig} for configuration lookup in runtime.
+ */
+public interface RealmConfigurationSource {
+  RealmConfigurationSource EMPTY_CONFIG = (rc, name) -> null;
+
+  static RealmConfigurationSource global(Map<String, ?> config) {

Review Comment:
   inlined



-- 
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-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org