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


Hi James,
thank you very much for picking up this change, greatly appreciated! I do have 
couple of high level notes and few comments:

1) It seems that the patch is having a lot of trailing white spaces (spaces, 
tabs at the end of lines), please do remove those.

2) It would be great to have an automated test to ensure that we won't 
accidentally regress from this. You've mentioned that you did manual testing - 
would you mind automating that?


src/java/org/apache/sqoop/mapreduce/DataDrivenImportJob.java
<https://reviews.apache.org/r/14779/#comment53098>

    Jar output directory is in /tmp if I'm not mistaken whereas both Hive 
create statement or generated java class are available in current working 
directory (CWD). Do you think that it would be feasible to store the schema in 
CWD to be consistent with the other generated files?



src/java/org/apache/sqoop/mapreduce/DataDrivenImportJob.java
<https://reviews.apache.org/r/14779/#comment53099>

    The behavior of other generated files in CWD is that they are not 
overriding anything and instead bailing out in case that the files already 
exists. Do you think that we can have the same behavior here to be consistent?


Jarcec

- Jarek Cecho


On Oct. 20, 2013, 8:10 p.m., James Anderson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14779/
> -----------------------------------------------------------
> 
> (Updated Oct. 20, 2013, 8:10 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-435
>     https://issues.apache.org/jira/browse/SQOOP-435
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> Automatically generate an Avro schema file (*.avsc) in the JAR output 
> directory when --as-avrodatafile is specified.
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/sqoop/mapreduce/DataDrivenImportJob.java 5afd90c 
> 
> Diff: https://reviews.apache.org/r/14779/diff/
> 
> 
> Testing
> -------
> 
> - Compared contents of *.avsc file against output from avro-tools getschema 
> operation.
> - Created an external Hive table with AvroSerDe, pointing avro.schema.url to 
> a copy of the schema indexed in ElasticSearch.
> 
> 
> Thanks,
> 
> James Anderson
> 
>

Reply via email to