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, 02 Jun 2013 14:07:39 GMT

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


Hi Raghav,
thank you very much for taking this one! I do have couple of nits:


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

    Nit: This change do not seem to be relevant for this patch.



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

    Nit: This change do not seem to be relevant for this patch.



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

    Nit: This change do not seem to be relevant for this patch.



src/java/org/apache/sqoop/tool/ImportTool.java
<https://reviews.apache.org/r/11517/#comment44177>

    It seems to me that using incremental import and this new argument should be mutually
exclusive as it do not make sense to do incremental import into directory that you will remove
at the start of the execution, right?


Jarcec

- Jarek Cecho


On May 31, 2013, 8:36 p.m., Raghav Gautam wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11517/
> -----------------------------------------------------------
> 
> (Updated May 31, 2013, 8:36 p.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/TestIncrementalImport.java cab77ec 
>   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