----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44115/#review121755 -----------------------------------------------------------
Thanks a lot for this work, Abe! A few comments as follows: 1. This JIRA add truststore at client side which makes it possible for client to specify the server it trusts. Is it also unnecessary to add a truststore at server side to let server specify the clients it trusts? 2. If #1 makes sense, then it's necessary to add keystore at client side. (This JIRA only makes it possible to specify the truststore at client side) shell/src/main/java/org/apache/sqoop/shell/SetServerFunction.java (line 20) <https://reviews.apache.org/r/44115/#comment183565> This import is unused. shell/src/main/java/org/apache/sqoop/shell/SetTruststoreFunction.java (line 84) <https://reviews.apache.org/r/44115/#comment183576> Variable trustFactory is not used. - Dian Fu On Feb. 29, 2016, 10:49 p.m., Abraham Fine wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44115/ > ----------------------------------------------------------- > > (Updated Feb. 29, 2016, 10:49 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-2844 > https://issues.apache.org/jira/browse/SQOOP-2844 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > SQOOP-2844: Sqoop2: TrustStore support for shell > > > Diffs > ----- > > common/src/main/java/org/apache/sqoop/utils/ProcessUtils.java PRE-CREATION > core/src/main/java/org/apache/sqoop/security/SecurityConstants.java > 91a1b8bb0b4a266381faccd11617bba236c6b446 > server/src/main/java/org/apache/sqoop/server/SqoopJettyServer.java > 60368afc0af8b899696476607a5c0bc6d6084794 > server/src/main/java/org/apache/sqoop/server/common/ServerError.java > a644e9f10ba792a9c36d5b90db1b5d7f29569e43 > shell/src/main/java/org/apache/sqoop/shell/SetCommand.java > 3feaac3061a8e6008ef021dff8022ea79a2c2ba7 > shell/src/main/java/org/apache/sqoop/shell/SetServerFunction.java > e430f9d13c8d6f7b637c923c3609e072582290b8 > shell/src/main/java/org/apache/sqoop/shell/SetTruststoreFunction.java > PRE-CREATION > shell/src/main/java/org/apache/sqoop/shell/ShellEnvironment.java > 36d0712505bd6d87fd7d332897f155f3cfbc5de7 > shell/src/main/java/org/apache/sqoop/shell/core/Constants.java > 8af53f21cb3a8a3555ca25fc65e2b6c5dfca3a2c > shell/src/main/resources/shell-resource.properties > 630c31d77a0a96525ef0f3a8ad20b64b5bb24c72 > test/src/main/java/org/apache/sqoop/test/infrastructure/SqoopTestCase.java > e3fbe315e2b23735e2e31321b038dd88c5f99021 > > test/src/test/java/org/apache/sqoop/integration/serverproperties/SslTest.java > 17503f3f24ce8344e96e6f7c60085f015eac38f7 > > Diff: https://reviews.apache.org/r/44115/diff/ > > > Testing > ------- > > yes. > > > Thanks, > > Abraham Fine > >
