tokoko commented on code in PR #3339:
URL: https://github.com/apache/polaris/pull/3339#discussion_r2653885493
##########
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:
I thought of doing something like this before. (using key to get the value
feels right). Having said that, I convinced myself that it wasn't worth it
because now you're using whole PolarisPrincipal object as part of the cache key
which contains more than just the name. I doubt using roles and properties will
cause cache misses all that much, but you're also relying on the fact that
someone in the future won't go ahead and write something more dynamic in there
(like a token which was discussed before iirc).
Or maybe I'm completely misunderstanding how this immutables library works...
--
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]
