Hi all, +1 on JB shape for the cleanup PR.
Properties bag on PolarisStorageConfigurationInfo for custom FileIO settings, and MinIO / S3Mock endpoint + pathStyleAccess on AwsStorageConfigurationInfo where those fields already exist. The test/dev fixtures that today put those on table-default.s3.* under SKIP_CREDENTIAL can move in the same PR. I'll take it that way once this settles. WDYT Dmitri? Thanks, *Vignesh* On Thu, 8 Oct 2026 at 09:47, Jean-Baptiste Onofré <[email protected]> wrote: > Hi all, > > I believe the core objective is clear and we all agree on it: server-side > FileIO should be constructed purely from StorageAccessConfig. Decoupling > FileIOFactory from catalog table-default.* is right from an architecture > standpoint. > > Now, regarding how we handle custom FileIO properties, I lean toward > Yufei's view over having StorageAccessConfigProvider scrape catalog-level > properties: > - StorageAccessConfigProvider also feeds client-side loadTable responses. > Pulling arbitrary catalog properties into StorageAccessConfig risks > vending internal/server-side properties to clients, and I don't think we > should have complex segregation to deal with that. > - filtering on properties based on prefix (e.g. S3 vs Azure) becomes fragle > once users bring custom FileIO implementations or generic settings (lke > HTTP client timeouts, retry handlers, ...). > - table-default.* in the Iceberg REST spec is specific for Iceberg > table-level defaults for clients. If a deployment needs custom storage > configuration for FileIO, that belongs on the storage configuration entity > itself. > > If we need to support FileIO customizations, I would prefer to add a > properties bag to PolarisStorageConfigurationInfo. It feels much cleaner > and keeps the responsibilities clear: > - catalog table-default.* remains strictly for client table defaults. > - storage configuration remains for all FileIO and credentals (server and > client). > > For setups (MiiniO, S3Mock, ...) where table-default.s3.* was previously > used as a workaround, AwsStorageConfigurationInfo already supports endpoint > and pathStyleAccess, so we can update there. > > Regards > JB > > > > On Wed, Oct 7, 2026 at 10:19 PM vignesh a <[email protected]> wrote: > > > Hi Dmitri, > > > > +1 on using only StorageAccessConfig for server-side FileIO. > > > > I'll take the cleanup PR once this settles. Test/dev paths that today > > put MinIO endpoint and keys on table-default.s3.* when AccessConfig is > > empty will need those on storage config instead. We can cover that in > > the PR. WDYT?? > > > > Thanks, > > *Vignesh* > > > > [5328] https://github.com/apache/polaris/pull/5328 > > > > On Wed, 7 Oct 2026 at 00:21, Dmitri Bourlatchkov <[email protected]> > wrote: > > > > > Cross-referencing related discussion: > > > > > > https://lists.apache.org/thread/49vsx3r803nmf35wsyxl2h3qvxfrrfgs > > > > > > On Tue, Oct 6, 2026 at 2:46 PM Dmitri Bourlatchkov <[email protected]> > > > wrote: > > > > > > > Hi All, > > > > > > > > This discussion stems [1] from PR [5328], but it is rooted in more > > > > fundamental Polaris behaviours. > > > > > > > > ATM the codebase uses a combination of StorageAccessConfig and > > _default_ > > > > table properties for constructing server-side FileIO objects. > > > > > > > > Note that it already deviates from what clients may use. > > > > > > > > Load table responses advertise FileIO-related properties in the REST > > > > API-level config section. These properties include vended credentials > > and > > > > endpoints. > > > > > > > > Generally, IRC config properties are sufficient for clients to > > construct > > > > operational FileIO objects to access storage files for tables loaded > > via > > > > Polaris. > > > > > > > > Therefore, I propose that Polaris code use only StorageAccessConfig > for > > > > constructing server-side FileIO objects. > > > > > > > > Thoughts? > > > > > > > > [1] > https://github.com/apache/polaris/pull/5328#discussion_r4184400135 > > > > > > > > [5328] https://github.com/apache/polaris/pull/5328 > > > > > > > > Cheers, > > > > Dmitri. > > > > > > > > > > > > > >
