dimas-b commented on code in PR #3224:
URL: https://github.com/apache/polaris/pull/3224#discussion_r2594524173
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java:
##########
@@ -123,6 +125,7 @@ public StorageAccessConfig getOrGenerateSubScopeCreds(
allowListOperation,
allowedReadLocations,
allowedWriteLocations,
+ polarisPrincipal,
Review Comment:
This naturally broadens the range of key values for _all_ storage provides,
but only STS use cases benefit from the value. Other storage backends will see
a reduction in cache efficiency without any change in credential vending
behaviour.
If you agree to my proposal of a feature flag, then even for STS the cache
efficiency may be reduced (when the flag is off) without any benefit.
WDYT?
I do not known of an easy solution for this, though. In my mind, ideally,
storage integrations implementation should provide a converter method,
condensing all available parameters to a sub-set of used parameters (based on
type and config), which would later be used for the cache key... however, this
will probably require a lot of refactoring.
--
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]
