You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Lars Francke <as...@lars-francke.de> on 2012/02/02 15:31:05 UTC
Re: Review Request: SQOOP-428: Support compression for Avro import
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3600/
-----------------------------------------------------------
(Updated 2012-02-02 14:31:05.367176)
Review request for Sqoop.
Changes
-------
Updated to remove any mention of Snappy (which isn't available on Hadoop 1.0.0)
Summary
-------
This basically only ports all the code from Avro's (1.5.4) AvroOutputFormat to the new MR API.
I've changed the test to extract the common functionality into a helper method because they are the same apart from the two command line arguments.
I could have deleted AvroJob completely but as I was told last time that binary compatibility needs to be maintained I left it in. It's not needed anymore as all necessary functionality can be gotten from Avro's own version of that file as far as I can tell. So if it's okay to delete that redundant file (two actually, cloudera and apache package) let me know and I'll provide a new patch.
This addresses bug SQOOP-428.
https://issues.apache.org/jira/browse/SQOOP-428
Diffs (updated)
-----
src/java/com/cloudera/sqoop/io/CodecMap.java ffe949b
src/java/org/apache/sqoop/io/CodecMap.java 5b67206
src/java/org/apache/sqoop/mapreduce/AvroJob.java a57aaf1
src/java/org/apache/sqoop/mapreduce/AvroOutputFormat.java 96befd7
src/java/org/apache/sqoop/mapreduce/ImportJobBase.java ed6954a
src/test/com/cloudera/sqoop/TestAvroImport.java 06834fc
src/test/com/cloudera/sqoop/io/TestCodecMap.java f2f4039
Diff: https://reviews.apache.org/r/3600/diff
Testing
-------
All tests pass for hadoopversion=20 but TestColumnTypes fails for me on 23. I can't see how that's related though.
Thanks,
Lars
Re: Review Request: SQOOP-428: Support compression for Avro import
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3600/#review4821
-----------------------------------------------------------
Ship it!
Thank you for your time spent on this issue sir.
- Jarek
On 2012-02-02 14:31:05, Lars Francke wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3600/
> -----------------------------------------------------------
>
> (Updated 2012-02-02 14:31:05)
>
>
> Review request for Sqoop.
>
>
> Summary
> -------
>
> This basically only ports all the code from Avro's (1.5.4) AvroOutputFormat to the new MR API.
>
> I've changed the test to extract the common functionality into a helper method because they are the same apart from the two command line arguments.
>
> I could have deleted AvroJob completely but as I was told last time that binary compatibility needs to be maintained I left it in. It's not needed anymore as all necessary functionality can be gotten from Avro's own version of that file as far as I can tell. So if it's okay to delete that redundant file (two actually, cloudera and apache package) let me know and I'll provide a new patch.
>
>
> This addresses bug SQOOP-428.
> https://issues.apache.org/jira/browse/SQOOP-428
>
>
> Diffs
> -----
>
> src/java/com/cloudera/sqoop/io/CodecMap.java ffe949b
> src/java/org/apache/sqoop/io/CodecMap.java 5b67206
> src/java/org/apache/sqoop/mapreduce/AvroJob.java a57aaf1
> src/java/org/apache/sqoop/mapreduce/AvroOutputFormat.java 96befd7
> src/java/org/apache/sqoop/mapreduce/ImportJobBase.java ed6954a
> src/test/com/cloudera/sqoop/TestAvroImport.java 06834fc
> src/test/com/cloudera/sqoop/io/TestCodecMap.java f2f4039
>
> Diff: https://reviews.apache.org/r/3600/diff
>
>
> Testing
> -------
>
> All tests pass for hadoopversion=20 but TestColumnTypes fails for me on 23. I can't see how that's related though.
>
>
> Thanks,
>
> Lars
>
>