Skip to content

Remove PolarisConfiguration.loadConfig (v2) #1858

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 11, 2025
Merged

Remove PolarisConfiguration.loadConfig (v2) #1858

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4e71b63
initial rebase
eric-maynard Jun 10, 2025
ce8af29
continue refactor
eric-maynard Jun 10, 2025
34407d4
stable
eric-maynard Jun 10, 2025
f4dc268
check in
eric-maynard Jun 11, 2025
18e24ab
stable
eric-maynard Jun 11, 2025
5040ce8
applying fixes from review
eric-maynard Jun 11, 2025
762b230
stable
eric-maynard Jun 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions ...ris/extension/persistence/impl/eclipselink/EclipseLinkPolarisMetaStoreManagerFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import jakarta.inject.Inject;
import java.nio.file.Path;
import org.apache.polaris.core.PolarisDiagnostics;
import org.apache.polaris.core.config.PolarisConfigurationStore;
import org.apache.polaris.core.context.RealmContext;
import org.apache.polaris.core.persistence.LocalPolarisMetaStoreManagerFactory;
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
Expand All @@ -46,12 +47,13 @@ public class EclipseLinkPolarisMetaStoreManagerFactory
@Inject PolarisStorageIntegrationProvider storageIntegrationProvider;

protected EclipseLinkPolarisMetaStoreManagerFactory() {
this(null);
this(null, null);
}

@Inject
protected EclipseLinkPolarisMetaStoreManagerFactory(PolarisDiagnostics diagnostics) {
super(diagnostics);
protected EclipseLinkPolarisMetaStoreManagerFactory(
PolarisDiagnostics diagnostics, PolarisConfigurationStore configurationStore) {
super(diagnostics, configurationStore);
}

@Override
Expand Down
8 changes: 6 additions & 2 deletions ...main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.apache.polaris.core.PolarisCallContext;
import org.apache.polaris.core.PolarisDefaultDiagServiceImpl;
import org.apache.polaris.core.PolarisDiagnostics;
import org.apache.polaris.core.config.PolarisConfigurationStore;
import org.apache.polaris.core.context.CallContext;
import org.apache.polaris.core.context.RealmContext;
import org.apache.polaris.core.entity.PolarisEntity;
Expand Down Expand Up @@ -74,6 +75,7 @@ public class JdbcMetaStoreManagerFactory implements MetaStoreManagerFactory {
@Inject PolarisStorageIntegrationProvider storageIntegrationProvider;
@Inject Instance<DataSource> dataSource;
@Inject RelationalJdbcConfiguration relationalJdbcConfiguration;
@Inject PolarisConfigurationStore configurationStore;

protected JdbcMetaStoreManagerFactory() {}

Expand Down Expand Up @@ -205,7 +207,8 @@ public synchronized StorageCredentialCache getOrCreateStorageCredentialCache(
RealmContext realmContext) {
if (!storageCredentialCacheMap.containsKey(realmContext.getRealmIdentifier())) {
storageCredentialCacheMap.put(
realmContext.getRealmIdentifier(), new StorageCredentialCache());
realmContext.getRealmIdentifier(),
new StorageCredentialCache(realmContext, configurationStore));
}

return storageCredentialCacheMap.get(realmContext.getRealmIdentifier());
Expand All @@ -216,7 +219,8 @@ public synchronized EntityCache getOrCreateEntityCache(RealmContext realmContext
if (!entityCacheMap.containsKey(realmContext.getRealmIdentifier())) {
PolarisMetaStoreManager metaStoreManager = getOrCreateMetaStoreManager(realmContext);
entityCacheMap.put(
realmContext.getRealmIdentifier(), new InMemoryEntityCache(metaStoreManager));
realmContext.getRealmIdentifier(),
new InMemoryEntityCache(realmContext, configurationStore, metaStoreManager));
}

return entityCacheMap.get(realmContext.getRealmIdentifier());
Expand Down
20 changes: 0 additions & 20 deletions polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.apache.polaris.core.context.CallContext;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -210,25 +209,6 @@ public BehaviorChangeConfiguration<T> buildBehaviorChangeConfiguration() {
}
}

/**
* Returns the value of a `PolarisConfiguration`, or the default if it cannot be loaded. This
* method does not need to be used when a `CallContext` is already available
*/
public static <T> T loadConfig(PolarisConfiguration<T> configuration) {
var callContext = CallContext.getCurrentContext();
if (callContext == null) {
LOGGER.warn(
String.format(
"Unable to load current call context; using %s = %s",
configuration.key, configuration.defaultValue));
return configuration.defaultValue;
}
return callContext
.getPolarisCallContext()
.getConfigurationStore()
.getConfiguration(callContext.getRealmContext(), configuration);
}

public static <T> Builder<T> builder() {
return new Builder<>();
}
Expand Down
20 changes: 13 additions & 7 deletions polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.apache.polaris.core.admin.model.StorageConfigInfo;
import org.apache.polaris.core.config.BehaviorChangeConfiguration;
import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
import org.apache.polaris.core.context.CallContext;
import org.apache.polaris.core.secrets.UserSecretReference;
import org.apache.polaris.core.storage.FileStorageConfigurationInfo;
import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
Expand Down Expand Up @@ -80,7 +81,7 @@ public static CatalogEntity of(PolarisBaseEntity sourceEntity) {
return null;
}

public static CatalogEntity fromCatalog(Catalog catalog) {
public static CatalogEntity fromCatalog(CallContext callContext, Catalog catalog) {
Builder builder =
new Builder()
.setName(catalog.getName())
Expand All @@ -90,7 +91,7 @@ public static CatalogEntity fromCatalog(Catalog catalog) {
internalProperties.put(CATALOG_TYPE_PROPERTY, catalog.getType().name());
builder.setInternalProperties(internalProperties);
builder.setStorageConfigurationInfo(
catalog.getStorageConfigInfo(), getDefaultBaseLocation(catalog));
callContext, catalog.getStorageConfigInfo(), getDefaultBaseLocation(catalog));
return builder.build();
}

Expand Down Expand Up @@ -247,7 +248,7 @@ public Builder setReplaceNewLocationPrefixWithCatalogDefault(String value) {
}

public Builder setStorageConfigurationInfo(
StorageConfigInfo storageConfigModel, String defaultBaseLocation) {
CallContext callContext, StorageConfigInfo storageConfigModel, String defaultBaseLocation) {
if (storageConfigModel != null) {
PolarisStorageConfigurationInfo config;
Set<String> allowedLocations = new HashSet<>(storageConfigModel.getAllowedLocations());
Expand All @@ -261,7 +262,7 @@ public Builder setStorageConfigurationInfo(
throw new BadRequestException("Must specify default base location");
}
allowedLocations.add(defaultBaseLocation);
validateMaxAllowedLocations(allowedLocations);
validateMaxAllowedLocations(callContext, allowedLocations);
switch (storageConfigModel.getStorageType()) {
case S3:
AwsStorageConfigInfo awsConfigModel = (AwsStorageConfigInfo) storageConfigModel;
Expand Down Expand Up @@ -305,10 +306,15 @@ public Builder setStorageConfigurationInfo(
}

/** Validate the number of allowed locations not exceeding the max value. */
private void validateMaxAllowedLocations(Collection<String> allowedLocations) {
Copy link
Contributor

@adutra adutra Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we need this validation anymore? Or is it being done elsewhere?

Copy link
Contributor Author

@eric-maynard eric-maynard Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a temporary BehaviorChangeConfiguration (the default is no validation) so I thought this might be a good time to remove it. but if you think we'd better separate out that change, I can try to re-add the check. Unfortunately, there's not a great narrow waist for adding a PolarisCallContext to an entity.

Copy link
Contributor

@adutra adutra Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, it's a BehaviorChangeConfiguration. OK to remove this validation then.

Copy link
Contributor Author

@eric-maynard eric-maynard Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I undid the change for now and piped a CallContext in, we can always remove it separately. Thanks for flagging this

private void validateMaxAllowedLocations(
CallContext callContext, Collection<String> allowedLocations) {
int maxAllowedLocations =
BehaviorChangeConfiguration.loadConfig(
BehaviorChangeConfiguration.STORAGE_CONFIGURATION_MAX_LOCATIONS);
callContext
.getPolarisCallContext()
.getConfigurationStore()
.getConfiguration(
callContext.getRealmContext(),
BehaviorChangeConfiguration.STORAGE_CONFIGURATION_MAX_LOCATIONS);
if (maxAllowedLocations != -1 && allowedLocations.size() > maxAllowedLocations) {
throw new IllegalArgumentException(
String.format(
Expand Down
2 changes: 1 addition & 1 deletion ...re/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1619,7 +1619,7 @@ private void revokeGrantRecord(
try {
EnumMap<StorageAccessProperty, String> creds =
storageIntegration.getSubscopedCreds(
callCtx.getDiagServices(),
callCtx,
storageConfigurationInfo,
allowListOperation,
allowedReadLocations,
Expand Down
13 changes: 10 additions & 3 deletions ...rc/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.apache.polaris.core.PolarisCallContext;
import org.apache.polaris.core.PolarisDefaultDiagServiceImpl;
import org.apache.polaris.core.PolarisDiagnostics;
import org.apache.polaris.core.config.PolarisConfigurationStore;
import org.apache.polaris.core.context.CallContext;
import org.apache.polaris.core.context.RealmContext;
import org.apache.polaris.core.entity.PolarisEntity;
Expand Down Expand Up @@ -63,10 +64,14 @@ public abstract class LocalPolarisMetaStoreManagerFactory<StoreType>
LoggerFactory.getLogger(LocalPolarisMetaStoreManagerFactory.class);

private final PolarisDiagnostics diagnostics;
private final PolarisConfigurationStore configurationStore;
private boolean bootstrap;

protected LocalPolarisMetaStoreManagerFactory(@Nonnull PolarisDiagnostics diagnostics) {
protected LocalPolarisMetaStoreManagerFactory(
@Nonnull PolarisDiagnostics diagnostics,
@Nonnull PolarisConfigurationStore configurationStore) {
this.diagnostics = diagnostics;
this.configurationStore = configurationStore;
}

protected abstract StoreType createBackingStore(@Nonnull PolarisDiagnostics diagnostics);
Expand Down Expand Up @@ -177,7 +182,8 @@ public synchronized StorageCredentialCache getOrCreateStorageCredentialCache(
RealmContext realmContext) {
if (!storageCredentialCacheMap.containsKey(realmContext.getRealmIdentifier())) {
storageCredentialCacheMap.put(
realmContext.getRealmIdentifier(), new StorageCredentialCache());
realmContext.getRealmIdentifier(),
new StorageCredentialCache(realmContext, configurationStore));
}

return storageCredentialCacheMap.get(realmContext.getRealmIdentifier());
Expand All @@ -188,7 +194,8 @@ public synchronized EntityCache getOrCreateEntityCache(RealmContext realmContext
if (!entityCacheMap.containsKey(realmContext.getRealmIdentifier())) {
PolarisMetaStoreManager metaStoreManager = getOrCreateMetaStoreManager(realmContext);
entityCacheMap.put(
realmContext.getRealmIdentifier(), new InMemoryEntityCache(metaStoreManager));
realmContext.getRealmIdentifier(),
new InMemoryEntityCache(realmContext, configurationStore, metaStoreManager));
}

return entityCacheMap.get(realmContext.getRealmIdentifier());
Expand Down
16 changes: 12 additions & 4 deletions ...ris-core/src/main/java/org/apache/polaris/core/persistence/cache/InMemoryEntityCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
import org.apache.polaris.core.PolarisCallContext;
import org.apache.polaris.core.config.BehaviorChangeConfiguration;
import org.apache.polaris.core.config.FeatureConfiguration;
import org.apache.polaris.core.config.PolarisConfiguration;
import org.apache.polaris.core.config.PolarisConfigurationStore;
import org.apache.polaris.core.context.RealmContext;
import org.apache.polaris.core.entity.PolarisBaseEntity;
import org.apache.polaris.core.entity.PolarisEntityType;
import org.apache.polaris.core.entity.PolarisGrantRecord;
Expand Down Expand Up @@ -58,7 +59,10 @@ public class InMemoryEntityCache implements EntityCache {
*
* @param polarisMetaStoreManager the meta store manager implementation
*/
public InMemoryEntityCache(@Nonnull PolarisMetaStoreManager polarisMetaStoreManager) {
public InMemoryEntityCache(
@Nonnull RealmContext realmContext,
@Nonnull PolarisConfigurationStore configurationStore,
@Nonnull PolarisMetaStoreManager polarisMetaStoreManager) {

// by name cache
this.byName = new ConcurrentHashMap<>();
Expand All @@ -76,15 +80,19 @@ public InMemoryEntityCache(@Nonnull PolarisMetaStoreManager polarisMetaStoreMana
};

long weigherTarget =
PolarisConfiguration.loadConfig(FeatureConfiguration.ENTITY_CACHE_WEIGHER_TARGET);
configurationStore.getConfiguration(
realmContext, FeatureConfiguration.ENTITY_CACHE_WEIGHER_TARGET);
Caffeine<Long, ResolvedPolarisEntity> byIdBuilder =
Caffeine.newBuilder()
.maximumWeight(weigherTarget)
.weigher(EntityWeigher.asWeigher())
.expireAfterAccess(1, TimeUnit.HOURS) // Expire entries after 1 hour of no access
.removalListener(removalListener); // Set the removal listener

if (PolarisConfiguration.loadConfig(BehaviorChangeConfiguration.ENTITY_CACHE_SOFT_VALUES)) {
boolean useSoftValues =
configurationStore.getConfiguration(
realmContext, BehaviorChangeConfiguration.ENTITY_CACHE_SOFT_VALUES);
if (useSoftValues) {
byIdBuilder.softValues();
}

Expand Down
2 changes: 1 addition & 1 deletion .../org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2053,7 +2053,7 @@ private PolarisEntityResolver resolveSecurableToRoleGrant(
try {
EnumMap<StorageAccessProperty, String> creds =
storageIntegration.getSubscopedCreds(
callCtx.getDiagServices(),
callCtx,
storageConfigurationInfo,
allowListOperation,
allowedReadLocations,
Expand Down
6 changes: 3 additions & 3 deletions polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageIntegration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import java.util.EnumMap;
import java.util.Map;
import java.util.Set;
import org.apache.polaris.core.PolarisDiagnostics;
import org.apache.polaris.core.context.CallContext;

/**
* Abstract of Polaris Storage Integration. It holds the reference to an object that having the
Expand All @@ -45,7 +45,7 @@ public String getStorageIdentifierOrId() {
/**
* Subscope the creds against the allowed read and write locations.
*
* @param diagnostics the diagnostics service
* @param callContext the call context
* @param storageConfig storage configuration
* @param allowListOperation whether to allow LIST on all the provided allowed read/write
* locations
Expand All @@ -54,7 +54,7 @@ public String getStorageIdentifierOrId() {
* @return An enum map including the scoped credentials
*/
public abstract EnumMap<StorageAccessProperty, String> getSubscopedCreds(
@Nonnull PolarisDiagnostics diagnostics,
@Nonnull CallContext callContext,
@Nonnull T storageConfig,
boolean allowListOperation,
@Nonnull Set<String> allowedReadLocations,
Expand Down
12 changes: 8 additions & 4 deletions ...e/src/main/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
package org.apache.polaris.core.storage.aws;

import static org.apache.polaris.core.config.FeatureConfiguration.STORAGE_CREDENTIAL_DURATION_SECONDS;
import static org.apache.polaris.core.config.PolarisConfiguration.loadConfig;

import jakarta.annotation.Nonnull;
import java.net.URI;
Expand All @@ -29,7 +28,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.stream.Stream;
import org.apache.polaris.core.PolarisDiagnostics;
import org.apache.polaris.core.context.CallContext;
import org.apache.polaris.core.storage.InMemoryStorageIntegration;
import org.apache.polaris.core.storage.StorageAccessProperty;
import org.apache.polaris.core.storage.StorageUtil;
Expand Down Expand Up @@ -63,11 +62,16 @@ public AwsCredentialsStorageIntegration(
/** {@inheritDoc} */
@Override
public EnumMap<StorageAccessProperty, String> getSubscopedCreds(
@Nonnull PolarisDiagnostics diagnostics,
@Nonnull CallContext callContext,
@Nonnull AwsStorageConfigurationInfo storageConfig,
boolean allowListOperation,
@Nonnull Set<String> allowedReadLocations,
@Nonnull Set<String> allowedWriteLocations) {
int storageCredentialDurationSeconds =
callContext
.getPolarisCallContext()
.getConfigurationStore()
.getConfiguration(callContext.getRealmContext(), STORAGE_CREDENTIAL_DURATION_SECONDS);
AssumeRoleRequest.Builder request =
AssumeRoleRequest.builder()
.externalId(storageConfig.getExternalId())
Expand All @@ -80,7 +84,7 @@ public EnumMap<StorageAccessProperty, String> getSubscopedCreds(
allowedReadLocations,
allowedWriteLocations)
.toJson())
.durationSeconds(loadConfig(STORAGE_CREDENTIAL_DURATION_SECONDS));
.durationSeconds(storageCredentialDurationSeconds);
credentialsProvider.ifPresent(
cp -> request.overrideConfiguration(b -> b.credentialsProvider(cp)));
AssumeRoleResponse response = stsClient.assumeRole(request.build());
Expand Down
12 changes: 8 additions & 4 deletions ...c/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
*/
package org.apache.polaris.core.storage.azure;

import static org.apache.polaris.core.config.FeatureConfiguration.STORAGE_CREDENTIAL_DURATION_SECONDS;

import com.azure.core.credential.AccessToken;
import com.azure.core.credential.TokenRequestContext;
import com.azure.identity.DefaultAzureCredential;
Expand Down Expand Up @@ -45,8 +47,7 @@
import java.util.HashSet;
import java.util.Objects;
import java.util.Set;
import org.apache.polaris.core.PolarisDiagnostics;
import org.apache.polaris.core.config.FeatureConfiguration;
import org.apache.polaris.core.context.CallContext;
import org.apache.polaris.core.storage.InMemoryStorageIntegration;
import org.apache.polaris.core.storage.StorageAccessProperty;
import org.slf4j.Logger;
Expand All @@ -71,7 +72,7 @@ public AzureCredentialsStorageIntegration() {

@Override
public EnumMap<StorageAccessProperty, String> getSubscopedCreds(
@Nonnull PolarisDiagnostics diagnostics,
@Nonnull CallContext callContext,
@Nonnull AzureStorageConfigurationInfo storageConfig,
boolean allowListOperation,
@Nonnull Set<String> allowedReadLocations,
Expand Down Expand Up @@ -126,7 +127,10 @@ public EnumMap<StorageAccessProperty, String> getSubscopedCreds(
// clock skew between the client and server,
OffsetDateTime startTime = start.truncatedTo(ChronoUnit.SECONDS).atOffset(ZoneOffset.UTC);
int intendedDurationSeconds =
FeatureConfiguration.loadConfig(FeatureConfiguration.STORAGE_CREDENTIAL_DURATION_SECONDS);
callContext
.getPolarisCallContext()
.getConfigurationStore()
.getConfiguration(callContext.getRealmContext(), STORAGE_CREDENTIAL_DURATION_SECONDS);
OffsetDateTime intendedEndTime =
start.plusSeconds(intendedDurationSeconds).atOffset(ZoneOffset.UTC);
OffsetDateTime maxAllowedEndTime =
Expand Down
Loading
Loading