You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Gwen Shapira <gs...@cloudera.com> on 2014/07/03 22:02:55 UTC
Re: 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/#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 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
>
>