sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jarek Cecho" <jar...@apache.org>
Subject Re: Review Request: Review request for SQOOP-883: Adds a --delete-target-dir option
Date Sun, 09 Jun 2013 15:17:58 GMT

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

Ship it!


Hi Raghav,
the patch do not longer applies on trunk, so would you mind rebasing it? Otherwise I do have
two small nits:


src/java/com/cloudera/sqoop/tool/BaseSqoopTool.java
<https://reviews.apache.org/r/11517/#comment44692>

    Nit: Please don't change files in com.cloudera modules. They are provided only for backward
compatibility with cope created for pre-1.4 versions and thus should not be altered anymore.



src/test/com/cloudera/sqoop/mapreduce/TestImportJob.java
<https://reviews.apache.org/r/11517/#comment44693>

    Nit: Do you think that it would make sense to also validate that the content of imported
directory is as expected or do you think that it's not needed in this case?


Jarcec

- Jarek Cecho


On June 6, 2013, 3:56 a.m., Raghav Gautam wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11517/
> -----------------------------------------------------------
> 
> (Updated June 6, 2013, 3:56 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Description
> -------
> 
> After first import, re-importing to a directory fails because the directory already exists.
This patch tries to fix that by providing a --delete-target-dir option. This option will delete
the target directory of the import before running the import if it exists.
> 
> 
> Diffs
> -----
> 
>   src/docs/user/import.txt ee10b1c 
>   src/java/com/cloudera/sqoop/tool/BaseSqoopTool.java a5f72f7 
>   src/java/org/apache/sqoop/SqoopOptions.java f18d43e 
>   src/java/org/apache/sqoop/tool/BaseSqoopTool.java 42f521f 
>   src/java/org/apache/sqoop/tool/ImportTool.java 2627726 
>   src/test/com/cloudera/sqoop/TestSqoopOptions.java c78cd87 
>   src/test/com/cloudera/sqoop/mapreduce/TestImportJob.java 6ab3b82 
> 
> Diff: https://reviews.apache.org/r/11517/diff/
> 
> 
> Testing
> -------
> 
> Added new test for the option.
> All the tests pass.
> 
> 
> Thanks,
> 
> Raghav Gautam
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message