Github user Ethanlm commented on a diff in the pull request: https://github.com/apache/storm/pull/2820#discussion_r215690394 --- Diff: storm-client/src/jvm/org/apache/storm/utils/NimbusClient.java --- @@ -22,74 +22,128 @@ import org.apache.storm.security.auth.ReqContext; import org.apache.storm.security.auth.ThriftClient; import org.apache.storm.security.auth.ThriftConnectionType; -import org.apache.storm.shade.com.google.common.collect.Lists; -import org.apache.storm.shade.org.apache.commons.lang.StringUtils; import org.apache.storm.thrift.transport.TTransportException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +/** + * Client used for connecting to nimbus. Typically you want to use a variant of the + * `getConfiguredClient` static method to get a client to use, as directly putting in + * a host and port does not support nimbus high availability. + */ public class NimbusClient extends ThriftClient { private static final Logger LOG = LoggerFactory.getLogger(NimbusClient.class); private static volatile Nimbus.Iface _localOverrideClient = null; private static String oldLeader = ""; /** * Indicates if this is a special client that is overwritten for local mode. */ - public final boolean _isLocal; - private Nimbus.Iface _client; + public final boolean isLocal; + private final Nimbus.Iface client; + /** + * Constructor, Please try to use `getConfiguredClient` instead of calling this directly. + * @param conf the conf for the client. + * @param host the host the client is to talk to. + * @param port the port the client is to talk to. + * @throws TTransportException on any error. + */ + @Deprecated public NimbusClient(Map<String, Object> conf, String host, int port) throws TTransportException { this(conf, host, port, null, null); } + /** + * Constructor, Please try to use `getConfiguredClient` instead of calling this directly. + * @param conf the conf for the client. + * @param host the host the client is to talk to. + * @param port the port the client is to talk to. + * @param timeout the timeout to use when connecting. + * @throws TTransportException on any error. + */ public NimbusClient(Map<String, Object> conf, String host, int port, Integer timeout) throws TTransportException { super(conf, ThriftConnectionType.NIMBUS, host, port, timeout, null); - _client = new Nimbus.Client(_protocol); - _isLocal = false; + client = new Nimbus.Client(_protocol); + isLocal = false; } + /** + * Constructor, Please try to use `getConfiguredClientAs` instead of calling this directly. + * @param conf the conf for the client. + * @param host the host the client is to talk to. + * @param port the port the client is to talk to. + * @param timeout the timeout to use when connecting. + * @param asUser the name of the user you want to impersonate (use with caution as it is not always supported). + * @throws TTransportException on any error. + */ public NimbusClient(Map<String, Object> conf, String host, Integer port, Integer timeout, String asUser) throws TTransportException { super(conf, ThriftConnectionType.NIMBUS, host, port, timeout, asUser); - _client = new Nimbus.Client(_protocol); - _isLocal = false; + client = new Nimbus.Client(_protocol); + isLocal = false; } + /** + * Constructor, Please try to use `getConfiguredClient` instead of calling this directly. + * @param conf the conf for the client. + * @param host the host the client is to talk to. + * @throws TTransportException on any error. + */ public NimbusClient(Map<String, Object> conf, String host) throws TTransportException { super(conf, ThriftConnectionType.NIMBUS, host, null, null, null); - _client = new Nimbus.Client(_protocol); - _isLocal = false; + client = new Nimbus.Client(_protocol); + isLocal = false; } private NimbusClient(Nimbus.Iface client) { super(new HashMap<>(), ThriftConnectionType.LOCAL_FAKE, "localhost", null, null, null); - _client = client; - _isLocal = true; + this.client = client; + isLocal = true; } /** + * Is the local override set or not. * @return true of new clients will be overridden to connect to a local cluster and not the configured remote cluster. */ public static boolean isLocalOverride() { return _localOverrideClient != null; } + /** + * Execute cb with a configured nimbus client that will be closed one cb returns. --- End diff -- nit: once
---