Github user laurentgo commented on a diff in the pull request:

    https://github.com/apache/drill/pull/578#discussion_r102347409
  
    --- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java ---
    @@ -88,22 +124,178 @@ public void submitQuery(UserResultsListener 
resultsListener, RunQuery query) {
         send(queryResultHandler.getWrappedListener(resultsListener), 
RpcType.RUN_QUERY, query, QueryId.class);
       }
     
    -  public void connect(RpcConnectionHandler<ServerConnection> handler, 
DrillbitEndpoint endpoint,
    -                      UserProperties props, UserBitShared.UserCredentials 
credentials) {
    +  public CheckedFuture<Void, RpcException> connect(DrillbitEndpoint 
endpoint, DrillProperties parameters,
    +                                                   UserCredentials 
credentials) {
    +    final FutureHandler handler = new FutureHandler();
         UserToBitHandshake.Builder hsBuilder = UserToBitHandshake.newBuilder()
             .setRpcVersion(UserRpcConfig.RPC_VERSION)
             .setSupportListening(true)
             .setSupportComplexTypes(supportComplexTypes)
             .setSupportTimeout(true)
             .setCredentials(credentials)
    -        .setClientInfos(UserRpcUtils.getRpcEndpointInfos(clientName));
    +        .setClientInfos(UserRpcUtils.getRpcEndpointInfos(clientName))
    +        .setSaslSupport(SaslSupport.SASL_AUTH)
    +        .setProperties(parameters.serializeForServer());
    +    this.properties = parameters;
    +
    +    
connectAsClient(queryResultHandler.getWrappedConnectionHandler(handler),
    +        hsBuilder.build(), endpoint.getAddress(), endpoint.getUserPort());
    +    return handler;
    +  }
     
    -    if (props != null) {
    -      hsBuilder.setProperties(props);
    +  /**
    +   * Check (after {@link #connect connecting}) if server requires 
authentication.
    +   *
    +   * @return true if server requires authentication
    +   */
    +  public boolean serverRequiresAuthentication() {
    +    return serverAuthMechanisms != null;
    +  }
    +
    +  /**
    +   * Returns a list of supported authentication mechanism. If called 
before {@link #connect connecting},
    +   * returns null. If called after {@link #connect connecting}, returns a 
list of supported mechanisms
    +   * iff authentication is required.
    +   *
    +   * @return list of supported authentication mechanisms
    +   */
    +  public List<String> getSupportedAuthenticationMechanisms() {
    +    return serverAuthMechanisms;
    +  }
    +
    +  /**
    +   * Authenticate to the server asynchronously. Returns a future that 
{@link CheckedFuture#checkedGet results}
    +   * in null if authentication succeeds, or throws a {@link SaslException} 
with relevant message if
    +   * authentication fails.
    +   *
    +   * This method uses properties provided at {@link #connect connection 
time} and override them with the
    +   * given properties, if any.
    +   *
    +   * @param overrides parameter overrides
    +   * @return result of authentication request
    +   */
    +  public CheckedFuture<Void, SaslException> authenticate(final 
DrillProperties overrides) {
    --- End diff --
    
    is there any need (other than for testing?) to not include authentication 
in the connection process?
    
    From my point of view, it should be included since the user already 
provided all the needed properties (overrides is NULL in DrillClient), and the 
user cannot do anything until authenticated anyway...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to