XJDKC commented on code in PR #724:
URL: https://github.com/apache/polaris/pull/724#discussion_r1920727471
##########
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:
For Step 2:
The only way a user can configure access to storage is by defining
`table-default.` properties at the catalog level. These properties are included
in the `tableProperties` argument of `refreshIOWithCredentials` and passed to
`FileIO`.
Since we are passing all the information that `refreshIOWithCredentials` has
to `FileIOFactory`, this ensures that `FileIOFactory` also receives the
`tableProperties` and can pass them to `FileIO`. Therefore, the case you
mentioned does not pose any issues.
Regarding the generic property bag approach: users can already set arbitrary
properties using the `table-default.` prefix. This functionality inherently
creates a generic property bag, which is outside the scope of this PR. To
restrict the properties customers can set by using this approach, we need
another PR. For the other info we pass to `FileIOFactory`, customers doesn't
have access to it and Polaris has the full control.
The new proposal is basically just trying to move the logic of
`refreshIOWithCredentials` to `FileIOFactory` which doesn't break any existing
feature. If we feel okay with the existing logic in `refreshIOWithCredentials`,
it should also be fine to move the logic in `FileIOFactory`.
--
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]
