You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Abraham Elmahrek <ab...@cloudera.com> on 2014/06/26 19:58:04 UTC
Review Request 23047: SQOOP-1138 incremental lastmodified should re-use
output directory
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23047/
-----------------------------------------------------------
Review request for Sqoop.
Bugs: HUE-1138
https://issues.apache.org/jira/browse/HUE-1138
Repository: sqoop-trunk
Description
-------
commit f141c60558f5d2519fb3bba53ca25d43cc4a35c9
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date: Thu Jun 26 10:24:10 2014 -0700
SQOOP-1138 incremental lastmodified should re-use output directory
Added "append mode" to lastmodified incremental options.
:100644 100644 6cbb873... e4b64f0... M src/java/org/apache/sqoop/tool/ImportTool.java
:100644 100644 8eadcdd... 14719a8... M src/test/com/cloudera/sqoop/TestIncrementalImport.java
Diffs
-----
src/java/org/apache/sqoop/tool/ImportTool.java 6cbb873
src/test/com/cloudera/sqoop/TestIncrementalImport.java 8eadcdd
Diff: https://reviews.apache.org/r/23047/diff/
Testing
-------
ran ant tests with new test.
Thanks,
Abraham Elmahrek
Re: Review Request 23047: SQOOP-1138 incremental lastmodified should re-use
output directory
Posted by Abraham Elmahrek <ab...@cloudera.com>.
> On June 26, 2014, 11:44 p.m., Kathleen Ting wrote:
> > Thanks Abe for picking this up. I suspect the incremental lastmodified mode has never lived up to the expected behavior of first merging old and new data together into the final output and then preserving only the last updated value for each row. There seems to be an expectation that SQOOP-1138 will do so but I’m not sure the current fix meets that. Thoughts?
Good point Kate, new review coming up.
- Abraham
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23047/#review46813
-----------------------------------------------------------
On June 26, 2014, 6:23 p.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23047/
> -----------------------------------------------------------
>
> (Updated June 26, 2014, 6:23 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1138
> https://issues.apache.org/jira/browse/SQOOP-1138
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> commit f141c60558f5d2519fb3bba53ca25d43cc4a35c9
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Thu Jun 26 10:24:10 2014 -0700
>
> SQOOP-1138 incremental lastmodified should re-use output directory
>
> Added "append mode" to lastmodified incremental options.
>
> :100644 100644 6cbb873... e4b64f0... M src/java/org/apache/sqoop/tool/ImportTool.java
> :100644 100644 8eadcdd... 14719a8... M src/test/com/cloudera/sqoop/TestIncrementalImport.java
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/tool/ImportTool.java 6cbb873
> src/test/com/cloudera/sqoop/TestIncrementalImport.java 8eadcdd
>
> Diff: https://reviews.apache.org/r/23047/diff/
>
>
> Testing
> -------
>
> ran ant tests with new test.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 23047: SQOOP-1138 incremental lastmodified should re-use
output directory
Posted by Kathleen Ting <ka...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23047/#review46813
-----------------------------------------------------------
Thanks Abe for picking this up. I suspect the incremental lastmodified mode has never lived up to the expected behavior of first merging old and new data together into the final output and then preserving only the last updated value for each row. There seems to be an expectation that SQOOP-1138 will do so but I’m not sure the current fix meets that. Thoughts?
- Kathleen Ting
On June 26, 2014, 6:23 p.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23047/
> -----------------------------------------------------------
>
> (Updated June 26, 2014, 6:23 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1138
> https://issues.apache.org/jira/browse/SQOOP-1138
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> commit f141c60558f5d2519fb3bba53ca25d43cc4a35c9
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Thu Jun 26 10:24:10 2014 -0700
>
> SQOOP-1138 incremental lastmodified should re-use output directory
>
> Added "append mode" to lastmodified incremental options.
>
> :100644 100644 6cbb873... e4b64f0... M src/java/org/apache/sqoop/tool/ImportTool.java
> :100644 100644 8eadcdd... 14719a8... M src/test/com/cloudera/sqoop/TestIncrementalImport.java
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/tool/ImportTool.java 6cbb873
> src/test/com/cloudera/sqoop/TestIncrementalImport.java 8eadcdd
>
> Diff: https://reviews.apache.org/r/23047/diff/
>
>
> Testing
> -------
>
> ran ant tests with new test.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 23047: SQOOP-1138 incremental lastmodified should re-use
output directory
Posted by Abraham Elmahrek <ab...@cloudera.com>.
> On June 28, 2014, 11:11 p.m., Jarek Cecho wrote:
> > Looks good, nice work Abe, couple of comments:
Thanks for the review Jarcec.
> On June 28, 2014, 11:11 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/tool/ImportTool.java, lines 1054-1055
> > <https://reviews.apache.org/r/23047/diff/2/?file=619737#file619737line1054>
> >
> > Shouldn't the conditions be other way around?
> >
> > E.g. if(mergeCol == NULL && incremetnalMode == DateLastModified) then bad else ok;
In the case mergeCol == NULL && incrementalMode == DataLastModified, it should continue executing the method to attempt to "move" the previously generated temporary directory to its final destination. If the destination exists already, it will fail (similar to exceptions seen prior to this review).
> On June 28, 2014, 11:11 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/tool/ImportTool.java, lines 453-454
> > <https://reviews.apache.org/r/23047/diff/2/?file=619737#file619737line453>
> >
> > Don't we also need to do clean up of the temporary directory? Like remove it or something.
Sure. This could be a lot of data and is an intermediate step.
> On June 28, 2014, 11:11 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/tool/ImportTool.java, line 34
> > <https://reviews.apache.org/r/23047/diff/2/?file=619737#file619737line34>
> >
> > Nit: Please don't use the asterisk import - always iterate over all imported classes.
Indeed.
- Abraham
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23047/#review46944
-----------------------------------------------------------
On June 27, 2014, 7:31 p.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23047/
> -----------------------------------------------------------
>
> (Updated June 27, 2014, 7:31 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1138
> https://issues.apache.org/jira/browse/SQOOP-1138
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> commit 7642758ec9d3e2d798bc18a73af2662783651594
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Thu Jun 26 14:48:31 2014 -0700
>
> SQOOP-1138 incremental lastmodified should re-use output directory
>
> Added merge-key option for lastmodified.
> If the target directory already exists, try to perform a merge.
> Otherwise, just move the temporary directory to the target
> directory.
>
> :100644 100644 6cbb873... bf654b5... M src/java/org/apache/sqoop/tool/ImportTool.java
> :100644 100644 8eadcdd... fd94552... M src/test/com/cloudera/sqoop/TestIncrementalImport.java
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/tool/ImportTool.java 6cbb873
> src/test/com/cloudera/sqoop/TestIncrementalImport.java 8eadcdd
>
> Diff: https://reviews.apache.org/r/23047/diff/
>
>
> Testing
> -------
>
> ran ant tests with new test.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 23047: SQOOP-1138 incremental lastmodified should re-use
output directory
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23047/#review46944
-----------------------------------------------------------
Looks good, nice work Abe, couple of comments:
src/java/org/apache/sqoop/tool/ImportTool.java
<https://reviews.apache.org/r/23047/#comment82510>
Nit: Please don't use the asterisk import - always iterate over all imported classes.
src/java/org/apache/sqoop/tool/ImportTool.java
<https://reviews.apache.org/r/23047/#comment82511>
Don't we also need to do clean up of the temporary directory? Like remove it or something.
src/java/org/apache/sqoop/tool/ImportTool.java
<https://reviews.apache.org/r/23047/#comment82512>
Shouldn't the conditions be other way around?
E.g. if(mergeCol == NULL && incremetnalMode == DateLastModified) then bad else ok;
Jarcec
- Jarek Cecho
On June 27, 2014, 7:31 p.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23047/
> -----------------------------------------------------------
>
> (Updated June 27, 2014, 7:31 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1138
> https://issues.apache.org/jira/browse/SQOOP-1138
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> commit 7642758ec9d3e2d798bc18a73af2662783651594
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Thu Jun 26 14:48:31 2014 -0700
>
> SQOOP-1138 incremental lastmodified should re-use output directory
>
> Added merge-key option for lastmodified.
> If the target directory already exists, try to perform a merge.
> Otherwise, just move the temporary directory to the target
> directory.
>
> :100644 100644 6cbb873... bf654b5... M src/java/org/apache/sqoop/tool/ImportTool.java
> :100644 100644 8eadcdd... fd94552... M src/test/com/cloudera/sqoop/TestIncrementalImport.java
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/tool/ImportTool.java 6cbb873
> src/test/com/cloudera/sqoop/TestIncrementalImport.java 8eadcdd
>
> Diff: https://reviews.apache.org/r/23047/diff/
>
>
> Testing
> -------
>
> ran ant tests with new test.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 23047: SQOOP-1138 incremental lastmodified should re-use
output directory
Posted by Abraham Elmahrek <ab...@cloudera.com>.
> On July 3, 2014, 8:02 p.m., Gwen Shapira wrote:
> > New concern: IIRC, the merge code doesn't work on Avro, but --incremental does. What will be the new behavior? Succeed in fetching the data but fail the merge? I'm not sure what's the correct behavior (except fixing merge to support Avro), but if we can fail early rather than after the Sqooping, it will be nicer to the users.
I did not know this. We can add logic to fail early.
- Abraham
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23047/#review47319
-----------------------------------------------------------
On June 30, 2014, 5:10 a.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23047/
> -----------------------------------------------------------
>
> (Updated June 30, 2014, 5:10 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1138
> https://issues.apache.org/jira/browse/SQOOP-1138
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> commit 7642758ec9d3e2d798bc18a73af2662783651594
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Thu Jun 26 14:48:31 2014 -0700
>
> SQOOP-1138 incremental lastmodified should re-use output directory
>
> Added merge-key option for lastmodified.
> If the target directory already exists, try to perform a merge.
> Otherwise, just move the temporary directory to the target
> directory.
>
> :100644 100644 6cbb873... bf654b5... M src/java/org/apache/sqoop/tool/ImportTool.java
> :100644 100644 8eadcdd... fd94552... M src/test/com/cloudera/sqoop/TestIncrementalImport.java
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/tool/ImportTool.java 6cbb873
> src/test/com/cloudera/sqoop/TestIncrementalImport.java 8eadcdd
>
> Diff: https://reviews.apache.org/r/23047/diff/
>
>
> Testing
> -------
>
> ran ant tests with new test.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 23047: SQOOP-1138 incremental lastmodified should re-use
output directory
Posted by Jarek Cecho <ja...@apache.org>.
> On July 3, 2014, 8:02 p.m., Gwen Shapira wrote:
> > New concern: IIRC, the merge code doesn't work on Avro, but --incremental does. What will be the new behavior? Succeed in fetching the data but fail the merge? I'm not sure what's the correct behavior (except fixing merge to support Avro), but if we can fail early rather than after the Sqooping, it will be nicer to the users.
>
> Abraham Elmahrek wrote:
> I did not know this. We can add logic to fail early.
Agreed, we should definitely fail as soon a possible. We do have section for catching unsupported combination of arguments in ImportTool, for example:
https://github.com/apache/sqoop/blob/trunk/src/java/org/apache/sqoop/tool/ImportTool.java#L876
So let's improve it?
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23047/#review47319
-----------------------------------------------------------
On June 30, 2014, 5:10 a.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23047/
> -----------------------------------------------------------
>
> (Updated June 30, 2014, 5:10 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1138
> https://issues.apache.org/jira/browse/SQOOP-1138
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> commit 7642758ec9d3e2d798bc18a73af2662783651594
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Thu Jun 26 14:48:31 2014 -0700
>
> SQOOP-1138 incremental lastmodified should re-use output directory
>
> Added merge-key option for lastmodified.
> If the target directory already exists, try to perform a merge.
> Otherwise, just move the temporary directory to the target
> directory.
>
> :100644 100644 6cbb873... bf654b5... M src/java/org/apache/sqoop/tool/ImportTool.java
> :100644 100644 8eadcdd... fd94552... M src/test/com/cloudera/sqoop/TestIncrementalImport.java
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/tool/ImportTool.java 6cbb873
> src/test/com/cloudera/sqoop/TestIncrementalImport.java 8eadcdd
>
> Diff: https://reviews.apache.org/r/23047/diff/
>
>
> Testing
> -------
>
> ran ant tests with new test.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 23047: SQOOP-1138 incremental lastmodified should re-use
output directory
Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23047/#review47319
-----------------------------------------------------------
New concern: IIRC, the merge code doesn't work on Avro, but --incremental does. What will be the new behavior? Succeed in fetching the data but fail the merge? I'm not sure what's the correct behavior (except fixing merge to support Avro), but if we can fail early rather than after the Sqooping, it will be nicer to the users.
- Gwen Shapira
On June 30, 2014, 5:10 a.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23047/
> -----------------------------------------------------------
>
> (Updated June 30, 2014, 5:10 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1138
> https://issues.apache.org/jira/browse/SQOOP-1138
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> commit 7642758ec9d3e2d798bc18a73af2662783651594
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Thu Jun 26 14:48:31 2014 -0700
>
> SQOOP-1138 incremental lastmodified should re-use output directory
>
> Added merge-key option for lastmodified.
> If the target directory already exists, try to perform a merge.
> Otherwise, just move the temporary directory to the target
> directory.
>
> :100644 100644 6cbb873... bf654b5... M src/java/org/apache/sqoop/tool/ImportTool.java
> :100644 100644 8eadcdd... fd94552... M src/test/com/cloudera/sqoop/TestIncrementalImport.java
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/tool/ImportTool.java 6cbb873
> src/test/com/cloudera/sqoop/TestIncrementalImport.java 8eadcdd
>
> Diff: https://reviews.apache.org/r/23047/diff/
>
>
> Testing
> -------
>
> ran ant tests with new test.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 23047: SQOOP-1138 incremental lastmodified should re-use
output directory
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23047/#review47359
-----------------------------------------------------------
Looks good to me - let's incorporate Gwen's suggestion and answer following question and let's get this in!
src/java/org/apache/sqoop/tool/ImportTool.java
<https://reviews.apache.org/r/23047/#comment83094>
I'm wondering whether we do want to create new and empty SqoopOptions object here or whether it would be better to clone the existing one. What do you think Abe?
Jarcec
- Jarek Cecho
On June 30, 2014, 5:10 a.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23047/
> -----------------------------------------------------------
>
> (Updated June 30, 2014, 5:10 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1138
> https://issues.apache.org/jira/browse/SQOOP-1138
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> commit 7642758ec9d3e2d798bc18a73af2662783651594
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Thu Jun 26 14:48:31 2014 -0700
>
> SQOOP-1138 incremental lastmodified should re-use output directory
>
> Added merge-key option for lastmodified.
> If the target directory already exists, try to perform a merge.
> Otherwise, just move the temporary directory to the target
> directory.
>
> :100644 100644 6cbb873... bf654b5... M src/java/org/apache/sqoop/tool/ImportTool.java
> :100644 100644 8eadcdd... fd94552... M src/test/com/cloudera/sqoop/TestIncrementalImport.java
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/tool/ImportTool.java 6cbb873
> src/test/com/cloudera/sqoop/TestIncrementalImport.java 8eadcdd
>
> Diff: https://reviews.apache.org/r/23047/diff/
>
>
> Testing
> -------
>
> ran ant tests with new test.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 23047: SQOOP-1138 incremental lastmodified should re-use
output directory
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23047/#review47763
-----------------------------------------------------------
Ship it!
Ship It!
- Jarek Cecho
On July 14, 2014, 8:25 p.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23047/
> -----------------------------------------------------------
>
> (Updated July 14, 2014, 8:25 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1138
> https://issues.apache.org/jira/browse/SQOOP-1138
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> commit 7642758ec9d3e2d798bc18a73af2662783651594
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Thu Jun 26 14:48:31 2014 -0700
>
> SQOOP-1138 incremental lastmodified should re-use output directory
>
> Added merge-key option for lastmodified.
> If the target directory already exists, try to perform a merge.
> Otherwise, just move the temporary directory to the target
> directory.
>
> :100644 100644 6cbb873... bf654b5... M src/java/org/apache/sqoop/tool/ImportTool.java
> :100644 100644 8eadcdd... fd94552... M src/test/com/cloudera/sqoop/TestIncrementalImport.java
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/tool/ImportTool.java 81904ac
> src/test/com/cloudera/sqoop/TestIncrementalImport.java 8eadcdd
>
> Diff: https://reviews.apache.org/r/23047/diff/
>
>
> Testing
> -------
>
> ran ant tests with new test.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 23047: SQOOP-1138 incremental lastmodified should re-use
output directory
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23047/
-----------------------------------------------------------
(Updated July 14, 2014, 8:25 p.m.)
Review request for Sqoop.
Changes
-------
Rebased against trunk.
Bugs: SQOOP-1138
https://issues.apache.org/jira/browse/SQOOP-1138
Repository: sqoop-trunk
Description
-------
commit 7642758ec9d3e2d798bc18a73af2662783651594
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date: Thu Jun 26 14:48:31 2014 -0700
SQOOP-1138 incremental lastmodified should re-use output directory
Added merge-key option for lastmodified.
If the target directory already exists, try to perform a merge.
Otherwise, just move the temporary directory to the target
directory.
:100644 100644 6cbb873... bf654b5... M src/java/org/apache/sqoop/tool/ImportTool.java
:100644 100644 8eadcdd... fd94552... M src/test/com/cloudera/sqoop/TestIncrementalImport.java
Diffs (updated)
-----
src/java/org/apache/sqoop/tool/ImportTool.java 81904ac
src/test/com/cloudera/sqoop/TestIncrementalImport.java 8eadcdd
Diff: https://reviews.apache.org/r/23047/diff/
Testing
-------
ran ant tests with new test.
Thanks,
Abraham Elmahrek
Re: Review Request 23047: SQOOP-1138 incremental lastmodified should re-use
output directory
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23047/
-----------------------------------------------------------
(Updated July 14, 2014, 6 p.m.)
Review request for Sqoop.
Changes
-------
Allow "append" mode with --incremental lastmodified.
Bugs: SQOOP-1138
https://issues.apache.org/jira/browse/SQOOP-1138
Repository: sqoop-trunk
Description
-------
commit 7642758ec9d3e2d798bc18a73af2662783651594
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date: Thu Jun 26 14:48:31 2014 -0700
SQOOP-1138 incremental lastmodified should re-use output directory
Added merge-key option for lastmodified.
If the target directory already exists, try to perform a merge.
Otherwise, just move the temporary directory to the target
directory.
:100644 100644 6cbb873... bf654b5... M src/java/org/apache/sqoop/tool/ImportTool.java
:100644 100644 8eadcdd... fd94552... M src/test/com/cloudera/sqoop/TestIncrementalImport.java
Diffs (updated)
-----
src/java/org/apache/sqoop/tool/ImportTool.java 6cbb873
src/test/com/cloudera/sqoop/TestIncrementalImport.java 8eadcdd
Diff: https://reviews.apache.org/r/23047/diff/
Testing
-------
ran ant tests with new test.
Thanks,
Abraham Elmahrek
Re: Review Request 23047: SQOOP-1138 incremental lastmodified should re-use
output directory
Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23047/#review47427
-----------------------------------------------------------
Ship it!
Ship It!
- Gwen Shapira
On July 8, 2014, 12:56 a.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23047/
> -----------------------------------------------------------
>
> (Updated July 8, 2014, 12:56 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1138
> https://issues.apache.org/jira/browse/SQOOP-1138
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> commit 7642758ec9d3e2d798bc18a73af2662783651594
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Thu Jun 26 14:48:31 2014 -0700
>
> SQOOP-1138 incremental lastmodified should re-use output directory
>
> Added merge-key option for lastmodified.
> If the target directory already exists, try to perform a merge.
> Otherwise, just move the temporary directory to the target
> directory.
>
> :100644 100644 6cbb873... bf654b5... M src/java/org/apache/sqoop/tool/ImportTool.java
> :100644 100644 8eadcdd... fd94552... M src/test/com/cloudera/sqoop/TestIncrementalImport.java
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/tool/ImportTool.java 6cbb873
> src/test/com/cloudera/sqoop/TestIncrementalImport.java 8eadcdd
>
> Diff: https://reviews.apache.org/r/23047/diff/
>
>
> Testing
> -------
>
> ran ant tests with new test.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 23047: SQOOP-1138 incremental lastmodified should re-use
output directory
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23047/#review47697
-----------------------------------------------------------
Looks good to me. I do see 4 test failures in TestIncrementalImport, could you take a look Abe?
- Jarek Cecho
On July 8, 2014, 12:56 a.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23047/
> -----------------------------------------------------------
>
> (Updated July 8, 2014, 12:56 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1138
> https://issues.apache.org/jira/browse/SQOOP-1138
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> commit 7642758ec9d3e2d798bc18a73af2662783651594
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Thu Jun 26 14:48:31 2014 -0700
>
> SQOOP-1138 incremental lastmodified should re-use output directory
>
> Added merge-key option for lastmodified.
> If the target directory already exists, try to perform a merge.
> Otherwise, just move the temporary directory to the target
> directory.
>
> :100644 100644 6cbb873... bf654b5... M src/java/org/apache/sqoop/tool/ImportTool.java
> :100644 100644 8eadcdd... fd94552... M src/test/com/cloudera/sqoop/TestIncrementalImport.java
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/tool/ImportTool.java 6cbb873
> src/test/com/cloudera/sqoop/TestIncrementalImport.java 8eadcdd
>
> Diff: https://reviews.apache.org/r/23047/diff/
>
>
> Testing
> -------
>
> ran ant tests with new test.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>
Re: Review Request 23047: SQOOP-1138 incremental lastmodified should re-use
output directory
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23047/
-----------------------------------------------------------
(Updated July 8, 2014, 12:56 a.m.)
Review request for Sqoop.
Bugs: SQOOP-1138
https://issues.apache.org/jira/browse/SQOOP-1138
Repository: sqoop-trunk
Description
-------
commit 7642758ec9d3e2d798bc18a73af2662783651594
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date: Thu Jun 26 14:48:31 2014 -0700
SQOOP-1138 incremental lastmodified should re-use output directory
Added merge-key option for lastmodified.
If the target directory already exists, try to perform a merge.
Otherwise, just move the temporary directory to the target
directory.
:100644 100644 6cbb873... bf654b5... M src/java/org/apache/sqoop/tool/ImportTool.java
:100644 100644 8eadcdd... fd94552... M src/test/com/cloudera/sqoop/TestIncrementalImport.java
Diffs (updated)
-----
src/java/org/apache/sqoop/tool/ImportTool.java 6cbb873
src/test/com/cloudera/sqoop/TestIncrementalImport.java 8eadcdd
Diff: https://reviews.apache.org/r/23047/diff/
Testing
-------
ran ant tests with new test.
Thanks,
Abraham Elmahrek
Re: Review Request 23047: SQOOP-1138 incremental lastmodified should re-use
output directory
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23047/
-----------------------------------------------------------
(Updated June 30, 2014, 5:10 a.m.)
Review request for Sqoop.
Changes
-------
Jarcec' feedback.
Bugs: SQOOP-1138
https://issues.apache.org/jira/browse/SQOOP-1138
Repository: sqoop-trunk
Description
-------
commit 7642758ec9d3e2d798bc18a73af2662783651594
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date: Thu Jun 26 14:48:31 2014 -0700
SQOOP-1138 incremental lastmodified should re-use output directory
Added merge-key option for lastmodified.
If the target directory already exists, try to perform a merge.
Otherwise, just move the temporary directory to the target
directory.
:100644 100644 6cbb873... bf654b5... M src/java/org/apache/sqoop/tool/ImportTool.java
:100644 100644 8eadcdd... fd94552... M src/test/com/cloudera/sqoop/TestIncrementalImport.java
Diffs (updated)
-----
src/java/org/apache/sqoop/tool/ImportTool.java 6cbb873
src/test/com/cloudera/sqoop/TestIncrementalImport.java 8eadcdd
Diff: https://reviews.apache.org/r/23047/diff/
Testing
-------
ran ant tests with new test.
Thanks,
Abraham Elmahrek
Re: Review Request 23047: SQOOP-1138 incremental lastmodified should re-use
output directory
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23047/
-----------------------------------------------------------
(Updated June 27, 2014, 7:31 p.m.)
Review request for Sqoop.
Bugs: SQOOP-1138
https://issues.apache.org/jira/browse/SQOOP-1138
Repository: sqoop-trunk
Description (updated)
-------
commit 7642758ec9d3e2d798bc18a73af2662783651594
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date: Thu Jun 26 14:48:31 2014 -0700
SQOOP-1138 incremental lastmodified should re-use output directory
Added merge-key option for lastmodified.
If the target directory already exists, try to perform a merge.
Otherwise, just move the temporary directory to the target
directory.
:100644 100644 6cbb873... bf654b5... M src/java/org/apache/sqoop/tool/ImportTool.java
:100644 100644 8eadcdd... fd94552... M src/test/com/cloudera/sqoop/TestIncrementalImport.java
Diffs
-----
src/java/org/apache/sqoop/tool/ImportTool.java 6cbb873
src/test/com/cloudera/sqoop/TestIncrementalImport.java 8eadcdd
Diff: https://reviews.apache.org/r/23047/diff/
Testing
-------
ran ant tests with new test.
Thanks,
Abraham Elmahrek
Re: Review Request 23047: SQOOP-1138 incremental lastmodified should re-use
output directory
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23047/
-----------------------------------------------------------
(Updated June 27, 2014, 7:24 p.m.)
Review request for Sqoop.
Changes
-------
Took kate's feedback into consideration and am performing a merge.
Bugs: SQOOP-1138
https://issues.apache.org/jira/browse/SQOOP-1138
Repository: sqoop-trunk
Description
-------
commit f141c60558f5d2519fb3bba53ca25d43cc4a35c9
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date: Thu Jun 26 10:24:10 2014 -0700
SQOOP-1138 incremental lastmodified should re-use output directory
Added "append mode" to lastmodified incremental options.
:100644 100644 6cbb873... e4b64f0... M src/java/org/apache/sqoop/tool/ImportTool.java
:100644 100644 8eadcdd... 14719a8... M src/test/com/cloudera/sqoop/TestIncrementalImport.java
Diffs (updated)
-----
src/java/org/apache/sqoop/tool/ImportTool.java 6cbb873
src/test/com/cloudera/sqoop/TestIncrementalImport.java 8eadcdd
Diff: https://reviews.apache.org/r/23047/diff/
Testing
-------
ran ant tests with new test.
Thanks,
Abraham Elmahrek
Re: Review Request 23047: SQOOP-1138 incremental lastmodified should re-use
output directory
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23047/
-----------------------------------------------------------
(Updated June 26, 2014, 6:23 p.m.)
Review request for Sqoop.
Bugs: SQOOP-1138
https://issues.apache.org/jira/browse/SQOOP-1138
Repository: sqoop-trunk
Description
-------
commit f141c60558f5d2519fb3bba53ca25d43cc4a35c9
Author: Abraham Elmahrek <ab...@elmahrek.com>
Date: Thu Jun 26 10:24:10 2014 -0700
SQOOP-1138 incremental lastmodified should re-use output directory
Added "append mode" to lastmodified incremental options.
:100644 100644 6cbb873... e4b64f0... M src/java/org/apache/sqoop/tool/ImportTool.java
:100644 100644 8eadcdd... 14719a8... M src/test/com/cloudera/sqoop/TestIncrementalImport.java
Diffs
-----
src/java/org/apache/sqoop/tool/ImportTool.java 6cbb873
src/test/com/cloudera/sqoop/TestIncrementalImport.java 8eadcdd
Diff: https://reviews.apache.org/r/23047/diff/
Testing
-------
ran ant tests with new test.
Thanks,
Abraham Elmahrek
Re: Review Request 23047: SQOOP-1138 incremental lastmodified should re-use
output directory
Posted by Gwen Shapira <gs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23047/#review46767
-----------------------------------------------------------
Ship it!
THANK YOU!!! This bug was so annoying. Thanks for nailing this!
- Gwen Shapira
On June 26, 2014, 5:58 p.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23047/
> -----------------------------------------------------------
>
> (Updated June 26, 2014, 5:58 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: HUE-1138
> https://issues.apache.org/jira/browse/HUE-1138
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> commit f141c60558f5d2519fb3bba53ca25d43cc4a35c9
> Author: Abraham Elmahrek <ab...@elmahrek.com>
> Date: Thu Jun 26 10:24:10 2014 -0700
>
> SQOOP-1138 incremental lastmodified should re-use output directory
>
> Added "append mode" to lastmodified incremental options.
>
> :100644 100644 6cbb873... e4b64f0... M src/java/org/apache/sqoop/tool/ImportTool.java
> :100644 100644 8eadcdd... 14719a8... M src/test/com/cloudera/sqoop/TestIncrementalImport.java
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/tool/ImportTool.java 6cbb873
> src/test/com/cloudera/sqoop/TestIncrementalImport.java 8eadcdd
>
> Diff: https://reviews.apache.org/r/23047/diff/
>
>
> Testing
> -------
>
> ran ant tests with new test.
>
>
> Thanks,
>
> Abraham Elmahrek
>
>