tokoko commented on code in PR #3339:
URL: https://github.com/apache/polaris/pull/3339#discussion_r2653926381
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheKey.java:
##########
@@ -53,7 +53,7 @@ public interface StorageCredentialCacheKey {
Optional<String> refreshCredentialsEndpoint();
@Value.Parameter(order = 8)
- Optional<String> principalName();
+ Optional<PolarisPrincipal> principal();
Review Comment:
okay, got it. I still think it's a ticking time bomb :laughing: especially
that it will affect cache even if only
`includePrincipalNameInSubscopedCredential` is activated.
I guess the only actual solution I see for these sorts of issues is to
delegate key generation to Integration classes themselves:
- Manager always loads Integration class (might have to introduce some sort
of a cache for Integration classes, maybe..)
- passes all the parameters and asks for a key
- checks the cache and returns if non-empty
- passes all the parameters again now asking for a value.
But that will need a major refactor that I'm still gonna get back to soon
:laughing: Until then, I guess it's to merge this not to block the feature.
--
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]
