adutra commented on code in PR #1397:
URL: https://github.com/apache/polaris/pull/1397#discussion_r2050623306
##########
service/common/src/main/java/org/apache/polaris/service/auth/Authenticator.java:
##########
@@ -20,8 +20,29 @@
import java.security.Principal;
import java.util.Optional;
+import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.iceberg.exceptions.ServiceFailureException;
+/**
+ * An interface for authenticating principals based on provided credentials.
+ *
+ * @param <C> the type of credentials used for authentication
+ * @param <P> the type of principal that is returned upon successful
authentication
+ */
public interface Authenticator<C, P extends Principal> {
- Optional<P> authenticate(C credentials);
+ /**
+ * Authenticates the given credentials and returns an optional principal.
+ *
+ * <p>If the credentials are not valid or if the authentication fails,
implementations may choose
Review Comment:
Again trying to document things a posteriori.
I don't really see the value of returning an `Optional` here, as the
semantics of returning "empty" conflict with that of throwing an error. But I
didn't want to change the method signature without knowing what others think.
--
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]
