dimas-b commented on code in PR #2736:
URL: https://github.com/apache/polaris/pull/2736#discussion_r2395789773
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java:
##########
@@ -832,13 +836,25 @@ public boolean sendNotification(
PolarisEntitySubType.ICEBERG_TABLE, identifier, notificationRequest);
}
+ /**
+ * Get access configuration for credential vending.
+ *
+ * @deprecated This method couples credential vending logic to the catalog
implementation,
+ * preventing reuse for federated catalogs. Use {@link
AccessConfigProvider#getAccessConfig}
+ * instead, which provides the same functionality as a standalone
factory. This method will be
+ * removed in a future release.
+ * @see AccessConfigProvider
+ */
@Override
+ @Deprecated
public AccessConfig getAccessConfig(
Review Comment:
This method has not callers in the Apache Polaris codebase. I think we
should remove it right away to simplify code maintenance.
This would be consistent with the current code evolution policy:
https://polaris.apache.org/releases/1.1.0/evolution/
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java:
##########
@@ -24,14 +24,22 @@
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.polaris.core.storage.AccessConfig;
import org.apache.polaris.core.storage.PolarisStorageActions;
+import org.apache.polaris.service.catalog.io.AccessConfigProvider;
/**
* Adds support for credential vending for (typically) {@link
org.apache.iceberg.TableOperations} to
* fetch access credentials that are inserted into the {@link
* org.apache.iceberg.rest.responses.LoadTableResponse#config()} property. See
the
* rest-catalog-open-api.yaml spec for details on the expected format of
vended credential
* configuration.
+ *
+ * @deprecated This interface tightly couples credential vending to catalog
implementation via
+ * inheritance, making it difficult to support credential vending for
federated catalogs or
+ * other non-Polaris catalog implementations. Use {@link
AccessConfigProvider} instead, which
+ * provides credential vending as a standalone factory that can work with
any catalog
+ * implementation. This interface will be removed in a future release.
*/
+@Deprecated
public interface SupportsCredentialDelegation {
AccessConfig getAccessConfig(
Review Comment:
Same here: there are no called in Apache Polaris code. Let's remove.
--
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]
