> On Aug. 22, 2014, 7:55 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/mapreduce/JobBase.java, line 186
> > <https://reviews.apache.org/r/24879/diff/2/?file=665034#file665034line186>
> >
> >     Could you please use SqoopOptions.getHiveHome() instead?
> >     
> >     
> > https://github.com/apache/sqoop/blob/trunk/src/java/org/apache/sqoop/SqoopOptions.java#L1303
> 
> richard zhou wrote:
>     Good suggestion.
>     I am just copying the way to get SQOOP_HOME in Line 171. I am wondering 
> why not use getSqoopHome() instead? Shall I create a JIRA to fix this or is 
> there any special reason?
>     
>     Line 171: String sqoopHome = System.getenv("SQOOP_HOME");

Good point Richard, let's open a follow up JIRA to fix that one as well.


> On Aug. 22, 2014, 7:55 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/mapreduce/JobBase.java, lines 194-195
> > <https://reviews.apache.org/r/24879/diff/2/?file=665034#file665034line194>
> >
> >     I don't think that throwing exception on missing HIVE_HOME is correct. 
> > What if the Hive libraries are already on our classpath because they have 
> > been added via different means (which would be the case when executed from 
> > Oozie).
> 
> richard zhou wrote:
>     Actually, I prefer to set this as a warning in my first patch. However, 
> Gwen Shapira pointed out that without Hive lib, the hive table will not be 
> created. But you are correct, there are some alternative to add hive libe 
> into classpath.
>     Additionally, if there does not have Hive lib, it will failed in 
> MapReduce process afterward as well.

I don't think that we can thrown an exception on missing HIVE_HOME 
environmental variable. But I see Gwen's point, it's better to fail early 
rather then late. What about detecting if some of the common Hive classes are 
on classpath and thrown an exception if we won't find them?


- Jarek


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24879/#review51311
-----------------------------------------------------------


On Aug. 25, 2014, 3:04 a.m., richard zhou wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24879/
> -----------------------------------------------------------
> 
> (Updated Aug. 25, 2014, 3:04 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1393
>     https://issues.apache.org/jira/browse/SQOOP-1393
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> https://issues.apache.org/jira/browse/SQOOP-1393
> 
> 
> Diffs
> -----
> 
>   ivy.xml e5334f155d4e9f85053aff11281ede0e49518e27 
>   ivy/libraries.properties cbcbf0dd11ff50e4047d86629b5d3e40dd949654 
>   src/java/org/apache/sqoop/mapreduce/DataDrivenImportJob.java 
> 300406abcdbe6759ed65e22bfad7f7617ee693a3 
>   src/java/org/apache/sqoop/mapreduce/JobBase.java 
> 032d4086be16b1b67e71514e0270f4a03b24ed71 
>   src/java/org/apache/sqoop/tool/CodeGenTool.java 
> a6d5dff9800ecacd913cd4f2e5262147740f988e 
>   src/java/org/apache/sqoop/tool/ImportTool.java 
> 54e618e583f72e5a5db5a852595eddb4a59394e5 
>   src/test/com/cloudera/sqoop/hive/TestHiveImport.java 
> 9c2a91c6efeeb21b85c04b7f8818aef84f23fd2a 
>   testdata/hive/scripts/normalImportAsParquet.q PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/24879/diff/
> 
> 
> Testing
> -------
> 
> testNormalHiveImportAsParquet() in TestHiveImport.java
> 
> 
> Thanks,
> 
> richard zhou
> 
>

Reply via email to