> On Oct. 22, 2014, 3:49 a.m., Veena Basavaraj wrote: > > core/src/main/java/org/apache/sqoop/authentication/AuthenticationConstants.java, > > line 27 > > <https://reviews.apache.org/r/26678/diff/4/?file=724268#file724268line27> > > > > is this for the sqoop.properties? is that what it means with sqoop > > configuration? since that term configuration is overloaded.
Yes, it is for sqoop.properties. > On Oct. 22, 2014, 3:49 a.m., Veena Basavaraj wrote: > > core/src/main/java/org/apache/sqoop/authentication/AuthenticationConstants.java, > > line 49 > > <https://reviews.apache.org/r/26678/diff/4/?file=724268#file724268line49> > > > > can we just have a static class, why do we need a constructor? Similar with ConfigurationConstants.java, disable explicit object creation. > On Oct. 22, 2014, 3:49 a.m., Veena Basavaraj wrote: > > core/src/main/java/org/apache/sqoop/authentication/AuthenticationError.java, > > line 24 > > <https://reviews.apache.org/r/26678/diff/4/?file=724269#file724269line24> > > > > why dont we move this unknown eror code to the base class, I see this > > c& p everywhere. can we have generic errors used across sqoop? > > > > Interested in knowign your thoughts > > https://issues.apache.org/jira/browse/SQOOP-1596 Good idea. Will create a JIRA for it. > On Oct. 22, 2014, 3:49 a.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? I have removed it, while I see these code everywhere... Driver.java, ConnectorManager.java... > On Oct. 22, 2014, 3:49 a.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 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:) > On Oct. 22, 2014, 3:49 a.m., Veena Basavaraj wrote: > > core/src/main/java/org/apache/sqoop/authentication/AuthenticationManager.java, > > line 104 > > <https://reviews.apache.org/r/26678/diff/4/?file=724272#file724272line104> > > > > since there is nothing here, why are even overriding it? > > can we remove it. Just leave the function, in case there is any logic needed in the future. > On Oct. 22, 2014, 3:49 a.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? The default value is SIMPLE. If uncommented, the KERBEROS will be used when starting sqoop server 2 - richard ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26678/#review57731 ----------------------------------------------------------- On Oct. 17, 2014, 8:53 a.m., richard zhou wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26678/ > ----------------------------------------------------------- > > (Updated Oct. 17, 2014, 8:53 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 > >
