singhpk234 commented on code in PR #13716:
URL: https://github.com/apache/iceberg/pull/13716#discussion_r2251820345
##########
core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java:
##########
@@ -992,6 +992,38 @@ private FileIO newFileIO(
}
}
+ private FileIO tableFileIO(
+ SessionContext context,
+ TableIdentifier identifier,
+ Map<String, String> config,
+ List<Credential> storageCredentials) {
+ // Check if either credential refresh is enabled from either client side
or server side.
+ // TODO: convert this to constants.
+ boolean s3RefreshEnabled =
+ PropertyUtil.propertyAsBoolean(config,
"client.refresh-credentials-enabled", false)
+ || PropertyUtil.propertyAsBoolean(
+ properties(), "client.refresh-credentials-enabled", false);
+ boolean adlsRefreshEnabled =
+ PropertyUtil.propertyAsBoolean(config,
"adls.refresh-credentials-enabled", false)
+ || PropertyUtil.propertyAsBoolean(
+ properties(), "adls.refresh-credentials-enabled", false);
+
+ // Inject the credentials refresh endpoint if, refresh is configured.
+ // This is done because the server would not know the complete URI of the
refresh endpoint.
Review Comment:
> why would the server not know the URI to refresh credentials for a given
table?
This is because server (maybe Polaris) which might be sitting behind a
series of micro-services post these hops when the call finally lands to server,
it doesn't know which uri the client used to reach the server :
Lets take an example :
client supplies :
> --conf spark.sql.catalog.<catalog_name>.uri=`<sub-domain>.domain.tld`
SDK creates the following URI before hitting LOAD table
> `<sub-domain>.domain.tld/v1/namespaces/<ns>/table/tbl`
Server would need to create
now we need to know this uri in the server end to create the table level
refresh-endpoint at server end
<sub-domain>.domain.tld/v1/namespaces/<ns>/table/<tbl>/credentials to the
client so that client can use it a server just can't send
`v1/namespaces/<ns>/table/<tbl>/credentials` because we need absolute uri,
never the less this fixed, hence was requesting if we can have this injected in
client end to support these use cases
> or return a different endpoint.
we would inject this only when no refresh endpoint is supplied by server
> The main point here is that only the server knows/controls what kind of
credentials to use for a particular table and how/whether to refresh them.
Server can still send the refresh enabled and endpoint this is just for
cases where server can't reconstruct it but just can say refresh is enabled and
then expects the client to create this uri on their own.
Never the less would also help for client override which at the moment can
just override it for one table for the whole catalog.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
