eric-maynard commented on code in PR #414:
URL: https://github.com/apache/polaris/pull/414#discussion_r1825034287
##########
polaris-service/src/main/java/org/apache/polaris/service/catalog/PolarisCatalogHandlerWrapper.java:
##########
@@ -834,7 +834,10 @@ public LoadTableResponse loadTableWithAccessDelegation(
callContext.getPolarisCallContext(),
catalogEntity,
PolarisConfiguration.ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING)) {
- throw new ForbiddenException("Access Delegation is not supported for
this catalog");
+ throw new ForbiddenException(
+ "Access Delegation is not enabled for this catalog. Please consult
applicable "
+ + "documentation for the catalog config property '%s' to enable
this feature",
+
PolarisConfiguration.ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING.catalogConfig());
Review Comment:
I see, it makes sense to avoid prompting users to blindly set a config.
Ideally we can convey some nuance here, but I don't think the
PolarisConfiguration descriptions are always adequate to do that. I'm concerned
about pushing the error messages into there because some errors occur due to an
interaction between configs or require other context.
In the future, we should look more closely at error messages but the
currently-proposed change seems beneficial for the time being.
--
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]
