> On March 20, 2018, 6:37 a.m., Rajat Khandelwal wrote: > > lens-driver-hive/src/main/java/org/apache/lens/driver/hive/RetryingThriftCLIServiceClientSasl.java > > Lines 125 (patched) > > <https://reviews.apache.org/r/66081/diff/1/?file=1976384#file1976384line125> > > > > This class is also redundant. Can we reuse > > `org.apache.hive.service.cli.thrift.RetryingThriftCLIServiceClient.InvocationResult`?
it is protected inner class, usage are scoped within package only. Wont be a good idea to follow package struture for single class in lens. cant even extend outside package, no default ctor available for InvocationResult in RetryingThriftCLIServiceClient. > On March 20, 2018, 6:37 a.m., Rajat Khandelwal wrote: > > lens-driver-hive/src/main/java/org/apache/lens/driver/hive/RetryingThriftCLIServiceClientSasl.java > > Lines 137-182 (patched) > > <https://reviews.apache.org/r/66081/diff/1/?file=1976384#file1976384line137> > > > > Both these methods can be easily avoided by simply extending > > `RetryingThriftCLIServiceClient`. it can't be extended form RetryingThriftCLIServiceClient please read above reply to puneet's comment. On March 20, 2018, 6:37 a.m., Ankit Kailaswar wrote: > > Current design: > > ```java > > HiveConf conf; > > if (conf.auth == Kerberose) { > > org.apache.lens.driver.hive.RetryingThriftCLIServiceClientSasl.newClient > > } else { > > > > org.apache.hive.service.cli.thrift.RetryingThriftCLIServiceClient.newClient > > } > > > > > > class RetryingThriftCLIServiceClientSasl { > > // some reuse from RetryingThriftCLIServiceClient > > // some code copied from RetryingThriftCLIServiceClient > > } > > > > ``` > > > > Proposal: > > > > > > ```java > > HiveConf conf; > > > > org.apache.lens.driver.hive.client.thrift.RetryingThriftCLIServiceClient.newClient > > > > class RetryingThriftCLIServiceClient extends > > org.apache.hive.service.cli.thrift.RetryingThriftCLIServiceClient { > > @Override > > protected synchronized TTransport connect(HiveConf conf) throws > > HiveSQLException, TTransportException { > > if (this.transport != null && this.transport.isOpen()) { > > this.transport.close(); > > } > > > > String host = conf.getVar(ConfVars.HIVE_SERVER2_THRIFT_BIND_HOST); > > int port = conf.getIntVar(ConfVars.HIVE_SERVER2_THRIFT_PORT); > > LOG.info("Connecting to " + host + ":" + port); > > this.transport = new TSocket(host, port); > > > > ((TSocket)this.transport).setTimeout((int)conf.getTimeVar(ConfVars.SERVER_READ_SOCKET_TIMEOUT, > > TimeUnit.SECONDS) * 1000); > > > > try { > > > > ((TSocket)this.transport).getSocket().setKeepAlive(conf.getBoolVar(ConfVars.SERVER_TCP_KEEP_ALIVE)); > > } catch (SocketException var8) { > > LOG.error("Error setting keep alive to " + > > conf.getBoolVar(ConfVars.SERVER_TCP_KEEP_ALIVE), var8); > > } > > /* Move the following code to new method getTransport below in > > the else part: > > String userName = > > conf.getVar(ConfVars.HIVE_SERVER2_THRIFT_CLIENT_USER); > > String passwd = > > conf.getVar(ConfVars.HIVE_SERVER2_THRIFT_CLIENT_PASSWORD); > > > > try { > > this.transport = PlainSaslHelper.getPlainTransport(userName, > > passwd, this.transport); > > } catch (SaslException var7) { > > LOG.error("Error creating plain SASL transport", var7); > > } > > */ > > this.transport = getTransport(conf); > > TProtocol protocol = new TBinaryProtocol(this.transport); > > this.transport.open(); > > this.base = new ThriftCLIServiceClient(new Client(protocol)); > > LOG.info("Connected!"); > > return this.transport; > > } > > protected TTransport getTransport(HiveConf conf) { > > if (conf.auth == Kerberose) { > > // use KerberoseSaslHelper > > } else { > > // use PlainSaslHelper > > } > > } > > } > > > > ``` member variables of base class cant be accesses from derived class. - Ankit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66081/#review199524 ----------------------------------------------------------- On March 20, 2018, 8:43 p.m., Ankit Kailaswar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66081/ > ----------------------------------------------------------- > > (Updated March 20, 2018, 8:43 p.m.) > > > Review request for lens, Amareshwari Sriramadasu, Rajat Khandelwal, and > Puneet Gupta. > > > Repository: lens > > > Description > ------- > > https://issues.apache.org/jira/browse/LENS-1506 > > This patch contains code changes to enable kerberos authentication for > 1. lens to hive > 2. lens to metastore > 3. lens to hdfs > > code changes are as follows, > 1. new http thrift client for hive driver to support sasl transport for > kerberozied hive server. > 2. cron to update KDC ticket before it expires. > > > Diffs > ----- > > lens-driver-hive/src/main/java/org/apache/lens/driver/hive/HiveDriver.java > 2eb94aa7 > > lens-driver-hive/src/main/java/org/apache/lens/driver/hive/RemoteThriftConnection.java > 54885f77 > > lens-driver-hive/src/main/java/org/apache/lens/driver/hive/RetryingThriftCLIServiceClientSasl.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java > d5273be8 > lens-server/src/main/java/org/apache/lens/server/BaseLensService.java > b9fcdd8b > > lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java > 31ac358d > > > Diff: https://reviews.apache.org/r/66081/diff/2/ > > > Testing > ------- > > unit testing > > > Thanks, > > Ankit Kailaswar > >
