dimas-b commented on code in PR #2759:
URL: https://github.com/apache/polaris/pull/2759#discussion_r2414056541
##########
runtime/service/src/main/java/org/apache/polaris/service/credentials/DefaultPolarisCredentialManager.java:
##########
@@ -78,32 +78,24 @@ public RealmContext getRealmContext() {
public @Nonnull ConnectionCredentials getConnectionCredentials(
@Nonnull ConnectionConfigInfoDpo connectionConfig) {
- // Select the appropriate vendor based on authentication type
AuthenticationType authType =
connectionConfig.getAuthenticationParameters().getAuthenticationType();
- Instance<ConnectionCredentialVendor> selectedVendor =
- credentialVendors.select(AuthType.Literal.of(authType));
- if (selectedVendor.isUnsatisfied()) {
- // TODO: Add credential vendors for other authentication types
- LOGGER.warn("No connection credential vendor found for authentication
type: {}", authType);
- return ConnectionCredentials.builder().build();
- }
-
- if (selectedVendor.isAmbiguous()) {
- LOGGER.error(
- "Multiple connection credential vendors found for authentication
type: {}. "
- + "Use @Priority to specify which vendor should be used. "
- + "Higher priority values take precedence.",
- authType);
+ // Use CDI to select the appropriate vendor based on the authentication
type
+ ConnectionCredentialVendor selectedVendor;
+ try {
+ selectedVendor =
credentialVendors.select(AuthType.Literal.of(authType)).get();
+ } catch (jakarta.enterprise.inject.UnsatisfiedResolutionException e) {
+ // No vendor registered for this authentication type
+ throw new IllegalStateException(
+ "No credential vendor available for authentication type: " +
authType, e);
+ } catch (jakarta.enterprise.inject.AmbiguousResolutionException e) {
Review Comment:
nit: I'd just catch the base exception. This is an internal error - we do
not need to report specific messages to the client. We should log the CDI
exception for trouble-shooting on the server side, but the client should
receive as generic error like `Unable to obtain credentials required for
executing this request`, IMHO.
--
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]
