tokoko commented on code in PR #4023:
URL: https://github.com/apache/polaris/pull/4023#discussion_r3012471227
##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java:
##########
@@ -538,4 +542,76 @@ default Optional<PrincipalRoleEntity>
findPrincipalRoleByName(
}
return Optional.of(entityResult.getEntity()).map(PrincipalRoleEntity::of);
}
+
+ /**
+ * Resolves the effective storage configuration for an entity by
reconstructing the entity
+ * hierarchy via {@code parentId} links and delegating to {@link
StorageConfigResolver#resolve},
+ * which is the single source of truth for storage-name override resolution.
+ *
+ * <p>This is the credential-vending counterpart of {@code
+ * FileIOUtil.resolveEffectiveStorageConfig}; both paths use {@link
StorageConfigResolver} to
+ * guarantee identical semantics.
+ *
+ * @param callCtx call context
+ * @param entity the entity whose storage config should be resolved
+ * @return the entity to use (either unchanged, or a synthetic copy with
resolved
+ * storageConfigInfo)
+ */
+ default PolarisBaseEntity resolveEntityStorageConfig(
+ @Nonnull PolarisCallContext callCtx, @Nonnull PolarisBaseEntity entity) {
+ Map<String, String> props = entity.getInternalPropertiesAsMap();
+
+ // Fast path: entity already carries the full storageConfigInfo (e.g.
catalog entities).
+ if
(props.containsKey(PolarisEntityConstants.getStorageConfigInfoPropertyName())) {
+ return entity;
+ }
+
+ // Build the entity chain (leaf-to-root) by walking parentId links.
+ long catalogId = entity.getCatalogId();
+ List<PolarisBaseEntity> chain = new ArrayList<>();
+ chain.add(entity);
+
+ long currentParentId = entity.getParentId();
+ while (currentParentId != PolarisEntityConstants.getNullId() &&
currentParentId != catalogId) {
+ EntityResult parentResult =
+ loadEntity(callCtx, catalogId, currentParentId,
PolarisEntityType.NAMESPACE);
Review Comment:
I think this PR would no longer have to solve this dual resolution issue
after #3699 since everything happens in `StorageAccessConfigProvider` there.
honestly this feels like we're putting bandaids on bandaids :smile: ideally we
should have a single place (StorageAccessConfigProvider most likely) that would
resolve storageInfo, resolve storage name, get additional properties from
config per storage name and return effective storage configuration. I don't
think that's possible to do now w/o first getting rid of all these unnecessary
layers.
--
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]
