dennishuo commented on code in PR #724:
URL: https://github.com/apache/polaris/pull/724#discussion_r1923230644
##########
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 believe this was just based on what's being discussed, perhaps @XJDKC
didn't want to push the change until we got to agreement on the comments here.
It sounds like we're actually in violent agreement though :)
From my comment earlier:
> In particular, we can still make the DefaultFileIOFactory effectively
enumerate the officially supported set of configuration that we'll be careful
as a project to ensure carries over from 1.0 to 2.0, etc. The "Default" polaris
service-runners (2a) will only be using such out-of-the-box factories, so we'll
be able to know exactly what we're allowed to evolve without breaking (2a).
> Those in the (2b) category are explicitly signing up to do their own
legwork if they both add plumbing of custom internal properties and use those
custom properties in a custom FileIOFactory, and I think what we're saying here
is that this extensibility mechanism is an explicit design choice to allow such
advanced use cases.
So whether we expose 1 or 2 plugin classes, the basic idea is that the
`FileIOFactory` at least already represents the main plugin entrypoint, and it
will effectively define exactly the fields used when constructing a FileIO
(whether or not it offloads this to an additional "`FileIOConfiguerer`" as you
suggest), and anyone using Polaris OSS will by default use the well-supported
DefaultFileIOFactory.
Only those who write additional custom code that uses custom properties will
go "off-road" so to speak, That's the (2b) category.
--
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]
