----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55401/#review166564 -----------------------------------------------------------
Hey Qiang, Thanks for opening this ticket and review, we as the Sqoop community greatly appreciate you'd like to improve the quality of the product. Although after I've reviewed your proposed changes I'm still uncerstain why these changes would be neccessary. As I can see the current solution is even more flexible compared to the one you're advising (so it gives the possibility to the user to set those path env variables one by one, and of course it has a meaningful fallback mechanism). Could you please provide me a bit more highlight why you'd like to do these changes? Thanks, Attila - Attila Szabo On Feb. 9, 2017, 7:16 a.m., Qiang Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55401/ > ----------------------------------------------------------- > > (Updated Feb. 9, 2017, 7:16 a.m.) > > > Review request for Sqoop, Andrew Bayer, Abraham Elmahrek, Abhijeet Gaikwad, > Abraham Fine, Ahmed Radwan, Arvind Prabhakar, Cheolsoo Park, Colin Ma, David > Robson, Dian Fu, Gwen Shapira, Hari Shreedharan, Jarek Cecho, Jonathan Hsieh, > Kathleen Ting, Attila Szabo, Mengwei Ding, Olivier Lamy, and Roman Shaposhnik. > > > Bugs: SQOOP-3102 > https://issues.apache.org/jira/browse/SQOOP-3102 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > $HADOOP_YARN_HOME and $HADOOP_HDFS_HOME were used as following in > start-all.sh of the hadoop: > # start hdfs daemons if hdfs is present > if [ -f "${HADOOP_HDFS_HOME}"/sbin/start-dfs.sh ]; then > "${HADOOP_HDFS_HOME}"/sbin/start-dfs.sh --config $HADOOP_CONF_DIR > fi > # start yarn daemons if yarn is present > if [ -f "${HADOOP_YARN_HOME}"/sbin/start-yarn.sh ]; then > "${HADOOP_YARN_HOME}"/sbin/start-yarn.sh --config $HADOOP_CONF_DIR > fi > Based on above logic the $HADOOP_YARN_HOME and $HADOOP_HDFS_HOME must equal > to $HADOOP_HOME. And they must be set as environment variable. > > In sqoop2 they were used as following in sqoop_server_classpath_set function > for sqoop.sh file. > HADOOP_COMMON_HOME=${HADOOP_COMMON_HOME:-${HADOOP_HOME}/share/hadoop/common} > HADOOP_HDFS_HOME=${HADOOP_HDFS_HOME:-${HADOOP_HOME}/share/hadoop/hdfs} > HADOOP_MAPRED_HOME=${HADOOP_MAPRED_HOME:-${HADOOP_HOME}/share/hadoop/mapreduce} > HADOOP_YARN_HOME=${HADOOP_YARN_HOME:-${HADOOP_HOME}/share/hadoop/yarn} > In above logic the HADOOP_HDFS_HOME will equal to HADOOP_HOME when > HADOOP_HDFS_HOME equal to $HADOOP_HOME in environment variable . The > HADOOP_YARN_HOME will equal to HADOOP_HOME when HADOOP_YARN_HOME was set to > $HADOOP_HOME in environment variable also. > As a result, sqoop2 start failure. > We shouldn't only check whether HADOOP_HDFS_HOME is empty, but also directly > set HADOOP_HDFS_HOME=${HADOOP_HOME}/share/hadoop/hdfs and > HADOOP_YARN_HOME={HADOOP_HOME}/share/hadoop/yarn in > sqoop_server_classpath_set function for sqoop.sh file. > > > Diffs > ----- > > dist/src/main/bin/sqoop.sh 3b2c6e1 > > Diff: https://reviews.apache.org/r/55401/diff/ > > > Testing > ------- > > > Thanks, > > Qiang Zhang > >
