XJDKC commented on code in PR #724:
URL: https://github.com/apache/polaris/pull/724#discussion_r1919377284
##########
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:
Yes, I would prefer using `PolarisStorageConfigurationInfo`.
Also, considering the point Eric mentioned, users may override the FileIO
impl, for example, they may want Polaris to use `HadoopFileIO` to access the
storage: see
[BasePolarisCatalog.java#L239-L257](https://github.com/apache/polaris/blob/be8cc359ce17eeab1f66a175092c8291b6703992/service/common/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java#L239-L257).
So passing `ioImplClassName` to FileIOFactory is necessary.
Since `PolarisStorageConfigurationInfo` is part of `internalProperties`, we
can pass `internalProperties` directly, (don't worry we may pass a bag to
`FileIO` since `FileIOFactory` will do the translation is owned by Polaris):
```java
public interface FileIOFactory {
FileIO loadFileIO(String impl, Map<String, String> properties, Map<String,
String> internalProperties);
}
public class CustomFileIOFactory {
FileIO loadFileIO(String impl, Map<String, String> properties, Map<String,
String> internalProperties) {
String proxyEndpoint = internalProperties.get(PROXY_ENDPOINT);
properties.add(S3FileIOProperties.ENDPOINT, proxyEndpoint); //
translation logic
return CatalogUtil.loadFileIO(impl, properties, new Configuration());
}
}
```
--
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]
