snazy commented on code in PR #2021:
URL: https://github.com/apache/polaris/pull/2021#discussion_r2196959527
##########
runtime/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java:
##########
@@ -465,7 +465,7 @@ public Supplier<TransactionalPersistence>
getOrCreateSessionSupplier(
@Override
public StorageCredentialCache
getOrCreateStorageCredentialCache(RealmContext realmContext) {
Review Comment:
If `RealmContext` is no longer needed, let's remove or at least
deprecate-for-removal this function and add one that takes no parameters.
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheKey.java:
##########
@@ -44,18 +42,13 @@ public class StorageCredentialCacheKey {
private final Set<String> allowedWriteLocations;
- /**
- * The callContext is passed to be used to fetch subscoped creds, but is not
used to hash/equals
- * as part of the cache key.
- */
- private @Nullable PolarisCallContext callContext;
Review Comment:
I suspect the removal of this field is the key of this PR?
It totally makes sense to not keep per-request state around.
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java:
##########
@@ -32,12 +32,19 @@ public class StorageCredentialCacheEntry {
public final EnumMap<StorageAccessProperty, String> credsMap;
private final ScopedCredentialsResult scopedCredentialsResult;
Review Comment:
Unrelated: this field is unused
--
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-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
