dimas-b commented on code in PR #4023:
URL: https://github.com/apache/polaris/pull/4023#discussion_r3376500905
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java:
##########
@@ -1937,6 +2002,88 @@ private String newTableMetadataFilePath(TableMetadata
meta, int newVersion) {
}
}
+ /**
+ * Process the user-facing {@link #POLARIS_STORAGE_NAME_PROPERTY} on a write
request: validate the
+ * value, gate against {@link
FeatureConfiguration#ALLOW_STORAGE_NAME_OVERRIDE} when the value
+ * would change, write the resolved value into the internal-property map (or
remove it when
+ * cleared), and strip the user-facing key from the user property map.
+ *
+ * <p>The flag is only enforced when the requested value would actually
mutate the persisted
+ * {@code storage_name_override}. An idempotent re-set with the prior value
is always allowed, so
+ * flipping the flag off does not break existing entities.
+ *
+ * <p>Property semantics:
+ *
+ * <ul>
+ * <li>{@code polaris.storage.name=foo} (and {@code foo} is non-blank,
valid) sets the override.
+ * <li>{@code polaris.storage.name=""} (or whitespace) clears the override.
+ * <li>Property absent from the user map: no change to the override.
+ * </ul>
+ *
+ * @param userProperties user-facing property map being persisted; the {@link
+ * #POLARIS_STORAGE_NAME_PROPERTY} key is removed in place if present.
+ * @param internalProperties internal-property map that will be persisted
onto the entity; the
+ * {@code storage_name_override} key is set or removed in place based on
the request.
+ * @param priorOverride the override value already persisted on the entity
(or {@code null} for a
+ * fresh create). Used to decide whether the request is an idempotent
re-set.
+ */
+ @VisibleForTesting
+ void processStorageNameOverrideOnWrite(
+ Map<String, String> userProperties,
+ Map<String, String> internalProperties,
+ @Nullable String priorOverride) {
Review Comment:
Is `priorOverride` actually an "override"? What does it override? 🤔
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java:
##########
@@ -1937,6 +2002,88 @@ private String newTableMetadataFilePath(TableMetadata
meta, int newVersion) {
}
}
+ /**
+ * Process the user-facing {@link #POLARIS_STORAGE_NAME_PROPERTY} on a write
request: validate the
+ * value, gate against {@link
FeatureConfiguration#ALLOW_STORAGE_NAME_OVERRIDE} when the value
+ * would change, write the resolved value into the internal-property map (or
remove it when
+ * cleared), and strip the user-facing key from the user property map.
+ *
+ * <p>The flag is only enforced when the requested value would actually
mutate the persisted
+ * {@code storage_name_override}. An idempotent re-set with the prior value
is always allowed, so
+ * flipping the flag off does not break existing entities.
+ *
+ * <p>Property semantics:
+ *
+ * <ul>
+ * <li>{@code polaris.storage.name=foo} (and {@code foo} is non-blank,
valid) sets the override.
+ * <li>{@code polaris.storage.name=""} (or whitespace) clears the override.
+ * <li>Property absent from the user map: no change to the override.
+ * </ul>
+ *
+ * @param userProperties user-facing property map being persisted; the {@link
+ * #POLARIS_STORAGE_NAME_PROPERTY} key is removed in place if present.
+ * @param internalProperties internal-property map that will be persisted
onto the entity; the
+ * {@code storage_name_override} key is set or removed in place based on
the request.
+ * @param priorOverride the override value already persisted on the entity
(or {@code null} for a
+ * fresh create). Used to decide whether the request is an idempotent
re-set.
+ */
+ @VisibleForTesting
+ void processStorageNameOverrideOnWrite(
+ Map<String, String> userProperties,
+ Map<String, String> internalProperties,
+ @Nullable String priorOverride) {
+ if (!userProperties.containsKey(POLARIS_STORAGE_NAME_PROPERTY)) {
+ return;
+ }
+ String rawValue = userProperties.remove(POLARIS_STORAGE_NAME_PROPERTY);
Review Comment:
Modifying parameters is not intuitive... Can we avoid this?
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/StorageConfigOverrideResolver.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.storage;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.polaris.core.entity.PolarisEntity;
+import org.apache.polaris.core.entity.PolarisEntityConstants;
+import org.jspecify.annotations.NonNull;
+
+/**
+ * Resolves the effective {@link PolarisStorageConfigurationInfo} for an
entity hierarchy by
+ * applying any nearest-ancestor {@code storage_name_override} on top of the
catalog's base storage
+ * configuration.
+ *
+ * <p>The input list is in root-to-leaf order (the catalog is at index 0). The
walk runs
+ * leaf-to-root and:
+ *
+ * <ol>
+ * <li>Captures the first {@code storage_name_override} encountered (nearest
wins).
+ * <li>Returns the first {@code storage_configuration_info} encountered —
the base config.
+ * </ol>
+ *
+ * <p>If a base config exists and an override was captured, the result is
{@link
+ * PolarisStorageConfigurationInfo#withStorageName} applied to the base. If no
base config exists,
+ * the result is empty regardless of any override.
+ */
+public final class StorageConfigOverrideResolver {
+
+ private StorageConfigOverrideResolver() {}
+
+ /**
+ * Walk {@code entityPath} leaf-to-root and return the effective storage
configuration. The base
+ * config is sourced from the nearest entity that has {@code
storage_configuration_info} in its
+ * internal properties; if any closer ancestor (including the leaf itself)
carries {@code
+ * storage_name_override}, that name replaces the base config's {@code
storageName}.
+ *
+ * @param entityPath entities in root-to-leaf order; the catalog is at index 0
+ * @return effective config, or empty if no entity in the chain has a base
storage config
+ */
+ public static Optional<PolarisStorageConfigurationInfo>
resolveEffectiveConfig(
+ @NonNull List<? extends PolarisEntity> entityPath) {
+ String override = null;
+ for (int i = entityPath.size() - 1; i >= 0; i--) {
+ Map<String, String> internalProps =
entityPath.get(i).getInternalPropertiesAsMap();
Review Comment:
nit: we assume indexed access is efficient here... WDYT about
`List.reversed()`?
##########
polaris-core/src/test/java/org/apache/polaris/core/storage/WithStorageNameTest.java:
##########
@@ -0,0 +1,154 @@
+/*
+ * 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.storage;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import org.apache.polaris.core.storage.aws.AwsStorageConfigurationInfo;
+import org.apache.polaris.core.storage.azure.AzureStorageConfigurationInfo;
+import org.apache.polaris.core.storage.gcp.GcpStorageConfigurationInfo;
+import org.junit.jupiter.api.Test;
+
+class WithStorageNameTest {
Review Comment:
This test class name is a bit confusing IMHO 🤔 Why not add the new test
cases to `PolarisStorageConfigurationInfoTest`?
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java:
##########
@@ -1937,6 +2002,88 @@ private String newTableMetadataFilePath(TableMetadata
meta, int newVersion) {
}
}
+ /**
+ * Process the user-facing {@link #POLARIS_STORAGE_NAME_PROPERTY} on a write
request: validate the
+ * value, gate against {@link
FeatureConfiguration#ALLOW_STORAGE_NAME_OVERRIDE} when the value
+ * would change, write the resolved value into the internal-property map (or
remove it when
+ * cleared), and strip the user-facing key from the user property map.
+ *
+ * <p>The flag is only enforced when the requested value would actually
mutate the persisted
+ * {@code storage_name_override}. An idempotent re-set with the prior value
is always allowed, so
+ * flipping the flag off does not break existing entities.
+ *
+ * <p>Property semantics:
+ *
+ * <ul>
+ * <li>{@code polaris.storage.name=foo} (and {@code foo} is non-blank,
valid) sets the override.
+ * <li>{@code polaris.storage.name=""} (or whitespace) clears the override.
+ * <li>Property absent from the user map: no change to the override.
+ * </ul>
+ *
+ * @param userProperties user-facing property map being persisted; the {@link
+ * #POLARIS_STORAGE_NAME_PROPERTY} key is removed in place if present.
+ * @param internalProperties internal-property map that will be persisted
onto the entity; the
+ * {@code storage_name_override} key is set or removed in place based on
the request.
+ * @param priorOverride the override value already persisted on the entity
(or {@code null} for a
+ * fresh create). Used to decide whether the request is an idempotent
re-set.
+ */
+ @VisibleForTesting
+ void processStorageNameOverrideOnWrite(
+ Map<String, String> userProperties,
+ Map<String, String> internalProperties,
+ @Nullable String priorOverride) {
+ if (!userProperties.containsKey(POLARIS_STORAGE_NAME_PROPERTY)) {
+ return;
+ }
+ String rawValue = userProperties.remove(POLARIS_STORAGE_NAME_PROPERTY);
+ String requested;
+ try {
+ requested = StorageNameValidator.normalizeAndValidate(rawValue);
+ } catch (IllegalArgumentException e) {
+ throw new BadRequestException(
+ e, "Invalid %s: %s", POLARIS_STORAGE_NAME_PROPERTY, e.getMessage());
+ }
+ boolean changing = !Objects.equal(priorOverride, requested);
+ if (changing &&
!realmConfig.getConfig(FeatureConfiguration.ALLOW_STORAGE_NAME_OVERRIDE)) {
+ throw new BadRequestException(
+ "Setting %s requires feature %s to be enabled",
+ POLARIS_STORAGE_NAME_PROPERTY,
FeatureConfiguration.ALLOW_STORAGE_NAME_OVERRIDE.key());
+ }
+ String storageOverrideKey =
PolarisEntityConstants.getStorageNameOverridePropertyName();
+ if (requested == null) {
+ internalProperties.remove(storageOverrideKey);
+ } else {
+ internalProperties.put(storageOverrideKey, requested);
Review Comment:
This logic effectively make the table metadata control the storage name
value on the server side. I'm not sure it is ideal.
Ultimately, the catalog owns storage configuration. Why should table
metadata have precedence over catalog-side settings?
I'd imagine Polaris CLI being the tool for setting storage name 🤔
Perhaps we need to dive deeper into this on the `dev` ML 🤔
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java:
##########
@@ -1937,6 +2002,88 @@ private String newTableMetadataFilePath(TableMetadata
meta, int newVersion) {
}
}
+ /**
+ * Process the user-facing {@link #POLARIS_STORAGE_NAME_PROPERTY} on a write
request: validate the
+ * value, gate against {@link
FeatureConfiguration#ALLOW_STORAGE_NAME_OVERRIDE} when the value
+ * would change, write the resolved value into the internal-property map (or
remove it when
+ * cleared), and strip the user-facing key from the user property map.
+ *
+ * <p>The flag is only enforced when the requested value would actually
mutate the persisted
+ * {@code storage_name_override}. An idempotent re-set with the prior value
is always allowed, so
+ * flipping the flag off does not break existing entities.
+ *
+ * <p>Property semantics:
+ *
+ * <ul>
+ * <li>{@code polaris.storage.name=foo} (and {@code foo} is non-blank,
valid) sets the override.
+ * <li>{@code polaris.storage.name=""} (or whitespace) clears the override.
+ * <li>Property absent from the user map: no change to the override.
+ * </ul>
+ *
+ * @param userProperties user-facing property map being persisted; the {@link
+ * #POLARIS_STORAGE_NAME_PROPERTY} key is removed in place if present.
+ * @param internalProperties internal-property map that will be persisted
onto the entity; the
+ * {@code storage_name_override} key is set or removed in place based on
the request.
+ * @param priorOverride the override value already persisted on the entity
(or {@code null} for a
+ * fresh create). Used to decide whether the request is an idempotent
re-set.
+ */
+ @VisibleForTesting
+ void processStorageNameOverrideOnWrite(
+ Map<String, String> userProperties,
+ Map<String, String> internalProperties,
+ @Nullable String priorOverride) {
+ if (!userProperties.containsKey(POLARIS_STORAGE_NAME_PROPERTY)) {
+ return;
+ }
+ String rawValue = userProperties.remove(POLARIS_STORAGE_NAME_PROPERTY);
+ String requested;
+ try {
+ requested = StorageNameValidator.normalizeAndValidate(rawValue);
+ } catch (IllegalArgumentException e) {
+ throw new BadRequestException(
Review Comment:
`IllegalArgumentException` should automatically result in a 400 response at
the REST API layer... Why do we need `BadRequestException` here?
##########
polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java:
##########
@@ -657,6 +657,18 @@ public static void enforceFeatureEnabledOrThrow(
.defaultValue(false)
.buildFeatureConfiguration();
+ public static final FeatureConfiguration<Boolean>
ALLOW_STORAGE_NAME_OVERRIDE =
+ PolarisConfiguration.<Boolean>builder()
+ .key("ALLOW_STORAGE_NAME_OVERRIDE")
+ .description(
+ "If set to true, allows namespaces and tables to set the
polaris.storage.name "
Review Comment:
What is the value of this flag? If a users points to a non-existent storage
name, the adverse effect is localized to that user's change.
Adding storage names is a Catalog Management action and should probably be
covered by AuthZ checks.
I'd think setting/changing `polaris.storage.name` probably needs an new
AuthZ operation rather than a global feature flag.
WDYT?
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java:
##########
@@ -18,35 +18,57 @@
*/
package org.apache.polaris.service.catalog.io;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
import java.util.Optional;
import org.apache.polaris.core.entity.PolarisEntity;
import org.apache.polaris.core.entity.PolarisEntityConstants;
import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper;
+import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
+import org.apache.polaris.core.storage.StorageConfigOverrideResolver;
public class FileIOUtil {
private FileIOUtil() {}
/**
- * Finds storage configuration information in the hierarchy of the resolved
storage entity.
+ * Finds storage configuration information in the hierarchy of the resolved
storage entity, with
+ * any nearest-ancestor {@code storage_name_override} applied to the
catalog's base config.
*
- * <p>This method starts at the "leaf" level (e.g., table) and walks
"upwards" through namespaces
- * in the hierarchy to the "root." It searches for the first entity
containing storage config
- * properties, identified using a key from {@link
- * PolarisEntityConstants#getStorageConfigInfoPropertyName()}.
+ * <p>This method walks the path leaf-to-root, locates the first entity
carrying {@code
+ * storage_configuration_info} (the catalog), and — if any closer ancestor
carries a {@code
+ * storage_name_override} — returns a synthetic copy of the base entity
whose serialized
+ * configuration has its {@code storageName} replaced. Callers that read the
resulting entity's
+ * internal properties (e.g., to stash on a purge task) therefore see the
effective config.
*
* @param resolvedStorageEntity the resolved entity wrapper containing the
hierarchical path
- * @return an {@link Optional} containing the entity with storage config, or
empty if not found
+ * @return an {@link Optional} containing the (possibly synthetic) entity
with the effective
+ * storage config, or empty if no entity in the chain has a base storage
config
*/
public static Optional<PolarisEntity> findStorageInfoFromHierarchy(
PolarisResolvedPathWrapper resolvedStorageEntity) {
- Optional<PolarisEntity> storageInfoEntity =
- resolvedStorageEntity.getRawFullPath().reversed().stream()
+ List<PolarisEntity> path = resolvedStorageEntity.getRawFullPath();
+ Optional<PolarisStorageConfigurationInfo> effective =
+ StorageConfigOverrideResolver.resolveEffectiveConfig(path);
+ if (effective.isEmpty()) {
+ return Optional.empty();
+ }
+ Optional<PolarisEntity> baseEntity =
+ path.reversed().stream()
.filter(
e ->
e.getInternalPropertiesAsMap()
.containsKey(PolarisEntityConstants.getStorageConfigInfoPropertyName()))
.findFirst();
- return storageInfoEntity;
+ if (baseEntity.isEmpty()) {
Review Comment:
Why repeat the search already performed by
`StorageConfigOverrideResolver.resolveEffectiveConfig()`? Can we be sure the
outcome is / will be the same?
Why not have `StorageConfigOverrideResolver` return a tuple of all the data
we need here?
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/StorageConfigOverrideResolver.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.storage;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.polaris.core.entity.PolarisEntity;
+import org.apache.polaris.core.entity.PolarisEntityConstants;
+import org.jspecify.annotations.NonNull;
+
+/**
+ * Resolves the effective {@link PolarisStorageConfigurationInfo} for an
entity hierarchy by
+ * applying any nearest-ancestor {@code storage_name_override} on top of the
catalog's base storage
+ * configuration.
+ *
+ * <p>The input list is in root-to-leaf order (the catalog is at index 0). The
walk runs
+ * leaf-to-root and:
+ *
+ * <ol>
+ * <li>Captures the first {@code storage_name_override} encountered (nearest
wins).
+ * <li>Returns the first {@code storage_configuration_info} encountered —
the base config.
+ * </ol>
+ *
+ * <p>If a base config exists and an override was captured, the result is
{@link
+ * PolarisStorageConfigurationInfo#withStorageName} applied to the base. If no
base config exists,
+ * the result is empty regardless of any override.
+ */
+public final class StorageConfigOverrideResolver {
+
+ private StorageConfigOverrideResolver() {}
+
+ /**
+ * Walk {@code entityPath} leaf-to-root and return the effective storage
configuration. The base
+ * config is sourced from the nearest entity that has {@code
storage_configuration_info} in its
+ * internal properties; if any closer ancestor (including the leaf itself)
carries {@code
+ * storage_name_override}, that name replaces the base config's {@code
storageName}.
+ *
+ * @param entityPath entities in root-to-leaf order; the catalog is at index 0
+ * @return effective config, or empty if no entity in the chain has a base
storage config
+ */
+ public static Optional<PolarisStorageConfigurationInfo>
resolveEffectiveConfig(
+ @NonNull List<? extends PolarisEntity> entityPath) {
+ String override = null;
+ for (int i = entityPath.size() - 1; i >= 0; i--) {
+ Map<String, String> internalProps =
entityPath.get(i).getInternalPropertiesAsMap();
+ if (override == null) {
+ String candidate =
+
internalProps.get(PolarisEntityConstants.getStorageNameOverridePropertyName());
+ if (candidate != null && !candidate.isEmpty()) {
+ override = candidate;
+ }
+ }
+ String serializedBase =
+
internalProps.get(PolarisEntityConstants.getStorageConfigInfoPropertyName());
+ if (serializedBase != null) {
+ PolarisStorageConfigurationInfo base =
+ PolarisStorageConfigurationInfo.deserialize(serializedBase);
+ if (override != null) {
+ return
Optional.of(PolarisStorageConfigurationInfo.withStorageName(base, override));
Review Comment:
I'm not sure I understand the purpose of reusing the same config with a
different name in different sub-trees 🤔 How do you envision this to be used?
My impression from the ML
[discussion](https://lists.apache.org/thread/nwcc8krhvm35olgc22676mm12cjkpgn5)
is that the idea was to define multiple distinct Storage Configurations at the
Catalog level and let entities choose one of them by name 🤔
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java:
##########
@@ -18,35 +18,57 @@
*/
package org.apache.polaris.service.catalog.io;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
import java.util.Optional;
import org.apache.polaris.core.entity.PolarisEntity;
import org.apache.polaris.core.entity.PolarisEntityConstants;
import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper;
+import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
+import org.apache.polaris.core.storage.StorageConfigOverrideResolver;
public class FileIOUtil {
private FileIOUtil() {}
/**
- * Finds storage configuration information in the hierarchy of the resolved
storage entity.
+ * Finds storage configuration information in the hierarchy of the resolved
storage entity, with
+ * any nearest-ancestor {@code storage_name_override} applied to the
catalog's base config.
*
- * <p>This method starts at the "leaf" level (e.g., table) and walks
"upwards" through namespaces
- * in the hierarchy to the "root." It searches for the first entity
containing storage config
- * properties, identified using a key from {@link
- * PolarisEntityConstants#getStorageConfigInfoPropertyName()}.
+ * <p>This method walks the path leaf-to-root, locates the first entity
carrying {@code
+ * storage_configuration_info} (the catalog), and — if any closer ancestor
carries a {@code
+ * storage_name_override} — returns a synthetic copy of the base entity
whose serialized
+ * configuration has its {@code storageName} replaced. Callers that read the
resulting entity's
+ * internal properties (e.g., to stash on a purge task) therefore see the
effective config.
*
* @param resolvedStorageEntity the resolved entity wrapper containing the
hierarchical path
- * @return an {@link Optional} containing the entity with storage config, or
empty if not found
+ * @return an {@link Optional} containing the (possibly synthetic) entity
with the effective
+ * storage config, or empty if no entity in the chain has a base storage
config
Review Comment:
Callers of this method only need to know the presence of this entity and if
present, its properties... Why bother with synthetic entities? I'd propose to
refactor this code to return an `Optional<some properties classs>`... WDYT?
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageConfigurationInfo.java:
##########
@@ -89,6 +92,44 @@ public boolean validatePrefix() {
public abstract StorageType getStorageType();
+ /**
+ * Return a copy of {@code base} with its {@code storageName} replaced by
{@code storageName}. All
+ * other fields are preserved. Used by the storage-name override path so
that an entity-level
+ * override flows through credential vending and into the per-backend cache
key without mutating
+ * the original config.
+ *
+ * @throws IllegalArgumentException if {@code base} is not one of the four
supported subtypes
+ */
+ public static PolarisStorageConfigurationInfo withStorageName(
+ PolarisStorageConfigurationInfo base, @Nullable String storageName) {
+ if (base instanceof AwsStorageConfigurationInfo aws) {
+ return ImmutableAwsStorageConfigurationInfo.builder()
Review Comment:
I'd propose to make an polymorphic `.toBuilder()` instance method and then
do `base.toBuilder().storageName(storageName).build()` without `instanceof`
checks.
--
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]
