gaborkaszab commented on code in PR #16536:
URL: https://github.com/apache/iceberg/pull/16536#discussion_r3306149649
##########
core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java:
##########
@@ -181,17 +181,37 @@ public RESTSessionCatalog() {
.uri(config.get(CatalogProperties.URI))
.withHeaders(RESTUtil.configHeaders(config))
.build(),
- null);
+ (FileIOBuilder) null);
}
public RESTSessionCatalog(
- Function<Map<String, String>, RESTClient> clientBuilder,
- BiFunction<SessionContext, Map<String, String>, FileIO> ioBuilder) {
+ Function<Map<String, String>, RESTClient> clientBuilder, FileIOBuilder
ioBuilder) {
Preconditions.checkNotNull(clientBuilder, "Invalid client builder: null");
this.clientBuilder = clientBuilder;
this.ioBuilder = ioBuilder;
}
+ /**
+ * @deprecated Use {@link #RESTSessionCatalog(Function, FileIOBuilder)}
instead.
+ */
+ @Deprecated
+ public RESTSessionCatalog(
+ Function<Map<String, String>, RESTClient> clientBuilder,
+ BiFunction<SessionContext, Map<String, String>, FileIO> ioBuilder) {
+ this(
+ clientBuilder,
+ ioBuilder == null
+ ? null
+ : (context, properties, credentials) -> {
+ FileIO fileIO = ioBuilder.apply(context, properties);
+ if (!credentials.isEmpty()
+ && fileIO instanceof SupportsStorageCredentials
ioWithCredentials) {
Review Comment:
Is the issue that the ioBuilder passed by Trino returns a FileIO that is not
an instance of SupportsStorageCredentials?
Isn't it the "contract" for FileIO that storage credentials can be set
through SupportsStorageCredentials.setCredentials()? How difficult would it be
to adjust the Trino side to work with this design?
##########
core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java:
##########
@@ -181,17 +181,37 @@ public RESTSessionCatalog() {
.uri(config.get(CatalogProperties.URI))
.withHeaders(RESTUtil.configHeaders(config))
.build(),
- null);
+ (FileIOBuilder) null);
}
public RESTSessionCatalog(
- Function<Map<String, String>, RESTClient> clientBuilder,
- BiFunction<SessionContext, Map<String, String>, FileIO> ioBuilder) {
+ Function<Map<String, String>, RESTClient> clientBuilder, FileIOBuilder
ioBuilder) {
Preconditions.checkNotNull(clientBuilder, "Invalid client builder: null");
this.clientBuilder = clientBuilder;
this.ioBuilder = ioBuilder;
}
+ /**
+ * @deprecated Use {@link #RESTSessionCatalog(Function, FileIOBuilder)}
instead.
Review Comment:
Please mention which is the target release for removal. Technically, in
core/ we can drop one minor release after deprecation. deprecation now is
released in 1.12.0, we can drop in 1.13.0. Unless we say this is considered
public API even though in core/
--
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]
