dimas-b commented on code in PR #724:
URL: https://github.com/apache/polaris/pull/724#discussion_r1915257405
##########
service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java:
##########
@@ -1627,6 +1627,12 @@ private FileIO refreshIOWithCredentials(
// the credentials should always override table-level properties, since
// storage configuration will be found at whatever entity defines it
tableProperties.putAll(credentialsMap);
+
+ // Populate the internal properties to FileIO in case the FileIO
implementation needs them
+ Map<String, String> internalProperties =
+
storageInfoEntity.map(PolarisEntity::getInternalPropertiesAsMap).orElse(Map.of());
Review Comment:
I agree with @snazy 's concern (from the main comment thread) about the
distinction between "properties" and "internal properties".
Currently, this is not well-defined in the code (e.g. there's no javadoc
explaining what is "internal"), so some clarification is probably called for.
That said, I think supporting customized access to storage is a valid use
case. We just need to pay more attention to defining the model for it so as to
avoid confusion in the future if/when related code need to be refactored.
@collado-mike : WDYT (as the original code contributor)?
--
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]
