Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Sqoop >> mail # dev >> Review Request: Review request for SQOOP-1035 "Add MS Sqoop Connecter tests to repo"


Copy link to this message
-
Re: Review Request: Review request for SQOOP-883: Adds a --delete-target-dir option


> On June 2, 2013, 2:07 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/tool/BaseSqoopTool.java, line 830
> > <https://reviews.apache.org/r/11517/diff/1/?file=299656#file299656line830>
> >
> >     Nit: This change do not seem to be relevant for this patch.

Removed. Thanks for pointing it out.
> On June 2, 2013, 2:07 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/tool/BaseSqoopTool.java, line 1054
> > <https://reviews.apache.org/r/11517/diff/1/?file=299656#file299656line1054>
> >
> >     Nit: This change do not seem to be relevant for this patch.

Removed. Thanks for pointing it out.
> On June 2, 2013, 2:07 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/tool/BaseSqoopTool.java, line 1060
> > <https://reviews.apache.org/r/11517/diff/1/?file=299656#file299656line1060>
> >
> >     Nit: This change do not seem to be relevant for this patch.

Removed. Thanks for pointing it out.
> On June 2, 2013, 2:07 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/tool/ImportTool.java, lines 720-724
> > <https://reviews.apache.org/r/11517/diff/1/?file=299657#file299657line720>
> >
> >     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?

Fixed. Thanks for pointing it out. I have also changed tests to reflect this.
- Raghav
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11517/#review21294
-----------------------------------------------------------
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
>
>