> On Oct. 21, 2014, 8:49 p.m., Veena Basavaraj wrote: > > Nice. Are there any tests pending?
Second nudge, are you planning on adding tests? > On Oct. 21, 2014, 8:49 p.m., Veena Basavaraj wrote: > > core/src/main/java/org/apache/sqoop/authentication/AuthenticationHandler.java, > > line 27 > > <https://reviews.apache.org/r/26678/diff/4/?file=724270#file724270line27> > > > > remove thse superflous comments, it is pretty clear that it is logger > > no? > > richard zhou wrote: > I have removed it, while I see these code everywhere... Driver.java, > ConnectorManager.java... yeah we should stop doing it at somepoint and clean the rest too. docs for the sake of docs is pretty cheesy. thanks for removing it, I apprecite it. > On Oct. 21, 2014, 8:49 p.m., Veena Basavaraj wrote: > > core/src/main/java/org/apache/sqoop/authentication/AuthenticationHandler.java, > > line 35 > > <https://reviews.apache.org/r/26678/diff/4/?file=724270#file724270line35> > > > > shoud not security enabled be on by default? can you please answer this? > On Oct. 21, 2014, 8:49 p.m., Veena Basavaraj wrote: > > core/src/main/java/org/apache/sqoop/authentication/AuthenticationManager.java, > > line 40 > > <https://reviews.apache.org/r/26678/diff/4/?file=724272#file724272line40> > > > > dont we need any thread safety around creation of these singletons? > > since I see a bunch of the methods been synchronized > > richard zhou wrote: > Actually, I am wondering why to write singletons like these. But, since > almost every Manager (RepositoryManager.java, ConnectorManager.java, ...) has > such mechanism, so I write the similar code:) same thing as above, I would encourage to ask why it is done the certian way no? If there is better pattern go for it. BTW, I am still waiting to know if these managers need to be thread safe? > On Oct. 21, 2014, 8:49 p.m., Veena Basavaraj wrote: > > dist/src/main/server/conf/sqoop.properties, line 142 > > <https://reviews.apache.org/r/26678/diff/4/?file=724276#file724276line142> > > > > commented out? are these defaults? > > richard zhou wrote: > The default value is SIMPLE. > If uncommented, the KERBEROS will be used when starting sqoop server 2 hope this is in the java doc somewhere - Veena ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26678/#review57731 ----------------------------------------------------------- On Oct. 22, 2014, 12:44 a.m., richard zhou wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26678/ > ----------------------------------------------------------- > > (Updated Oct. 22, 2014, 12:44 a.m.) > > > Review request for Sqoop. > > > Repository: sqoop-sqoop2 > > > Description > ------- > > Kerberos support when starting service > > > Diffs > ----- > > core/pom.xml 2b6e436d637eb03493323e5b36e31e433c1f8bbb > > core/src/main/java/org/apache/sqoop/authentication/AuthenticationConstants.java > PRE-CREATION > core/src/main/java/org/apache/sqoop/authentication/AuthenticationError.java > PRE-CREATION > > core/src/main/java/org/apache/sqoop/authentication/AuthenticationHandler.java > PRE-CREATION > > core/src/main/java/org/apache/sqoop/authentication/AuthenticationHandlerFactory.java > PRE-CREATION > > core/src/main/java/org/apache/sqoop/authentication/AuthenticationManager.java > PRE-CREATION > > core/src/main/java/org/apache/sqoop/authentication/KerberosAuthenticationHandler.java > PRE-CREATION > > core/src/main/java/org/apache/sqoop/authentication/SimpleAuthenticationHandler.java > PRE-CREATION > core/src/main/java/org/apache/sqoop/core/SqoopServer.java > ac836c7cee010144696ab17645ccd008aed5762d > dist/src/main/server/conf/sqoop.properties > bb010166120321899425f84edb8e1ad6512626d2 > pom.xml f25a29f6db673e6080dcd5ccd51bab76ab38bff4 > > Diff: https://reviews.apache.org/r/26678/diff/ > > > Testing > ------- > > > Thanks, > > richard zhou > >
