adutra commented on code in PR #3750:
URL: https://github.com/apache/polaris/pull/3750#discussion_r2918976358
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/DefaultAccessDelegationModeResolver.java:
##########
@@ -129,22 +125,6 @@ private AccessDelegationMode
resolveVendedCredentialsVsRemoteSigning(
return VENDED_CREDENTIALS;
}
- // Check if credential vending is enabled for this catalog.
Review Comment:
Why was this check removed?
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -478,10 +478,12 @@ public LoadTableResponse createTableDirect(
authorizeCreateTableDirect(namespace, request, delegationModes);
// Resolve the optimal delegation mode based on catalog capabilities
(after authorization)
- AccessDelegationMode resolvedMode =
resolveAccessDelegationModes(delegationModes);
-
- // Check if external catalog access delegation is allowed for the resolved
mode
- checkAllowExternalCatalogAccessDelegation(resolvedMode);
+ AccessDelegationMode resolvedMode = AccessDelegationMode.UNKNOWN;
Review Comment:
I suppose this new change is a consequence of my suggestion to throw an
exception from the resolver rather than returning `UNKNOWN`.
Unfortunately, I think this is becoming a bit too verbose. I apologize for
having make that suggestion. Let me think about this some more.
--
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]
