----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40625/#review108340 -----------------------------------------------------------
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsToInitializer.java (line 62) <https://reviews.apache.org/r/40625/#comment167768> creating the proxy user and then generating the delegation tokens is a pattern that we use twice. once in the hdfstoinitializer and again in the hdfsfrominitializer. perhaps, it could make sense to do something similar to what is being done when we load the delegation tokens (`createProxyUserAndLoadDelegationTokens`) and have a method `createProxyUserAndGenerateDelegationTokens`). that way we can make `generateDelegationTokens` private, and never have to worry about it being called outside of a delegation block. connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/security/SecurityUtils.java (line 55) <https://reviews.apache.org/r/40625/#comment167771> i understand that we used `getLoginUser` previously. is this the correct way to go instead of using `getCurrentUser`? connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/security/SecurityUtils.java (line 84) <https://reviews.apache.org/r/40625/#comment167766> how verbose is this, maybe it should be a debug? connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/security/TestSecurityUtils.java (line 27) <https://reviews.apache.org/r/40625/#comment167770> can we test writing the tokens to and reading them from the context? - Abraham Fine On Nov. 24, 2015, 12:58 a.m., Jarek Cecho wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40625/ > ----------------------------------------------------------- > > (Updated Nov. 24, 2015, 12:58 a.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-2709 > https://issues.apache.org/jira/browse/SQOOP-2709 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > I've provided util class that can retrieve delegation token for "current" > user and store it in our Context that is passed to execution engine. > > > Diffs > ----- > > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConstants.java > 39ee4a3 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java > 583acdd > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsFromInitializer.java > be837ca > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java > 04acd18 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsPartitioner.java > 998b903 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsToDestroyer.java > 2bad23a > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsToInitializer.java > 5856371 > > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/security/SecurityUtils.java > PRE-CREATION > > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/security/TestSecurityUtils.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/40625/diff/ > > > Testing > ------- > > I've tested the patch on secured real cluster to make sure that it's working. > Sadly I did not included any integration test as our suite currently doesn't > have any support for MiniKDC (this is something that we will add later). > > > Thanks, > > Jarek Cecho > >
