You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Szabolcs Vasas <va...@gmail.com> on 2016/12/06 13:54:52 UTC
Review Request 54421: Enhance error (tool.ImportTool: Encountered
IOException running import job: java.io.IOException: Expected schema) to
suggest workaround (--map-column-java)
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54421/
-----------------------------------------------------------
Review request for Sqoop.
Bugs: SQOOP-3068
https://issues.apache.org/jira/browse/SQOOP-3068
Repository: sqoop-trunk
Description
-------
Enhance error (tool.ImportTool: Encountered IOException running import job: java.io.IOException: Expected schema) to suggest workaround (--map-column-java)
Diffs
-----
src/java/org/apache/sqoop/avro/AvroSchemaMismatchException.java PRE-CREATION
src/java/org/apache/sqoop/mapreduce/ParquetJob.java b077d9b
src/java/org/apache/sqoop/tool/ImportTool.java ed951ea
src/test/com/cloudera/sqoop/hive/TestHiveImport.java 26d087b
src/test/org/apache/sqoop/tool/TestImportTool.java 4136e9f
src/test/org/apache/sqoop/util/ExpectedLogMessage.java PRE-CREATION
Diff: https://reviews.apache.org/r/54421/diff/
Testing
-------
New integration test and unit test is added.
Thanks,
Szabolcs Vasas
Re: Review Request 54421: Enhance error (tool.ImportTool: Encountered
IOException running import job: java.io.IOException: Expected schema)
to suggest workaround (--map-column-java)
Posted by Boglarka Egyed <eg...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54421/#review159017
-----------------------------------------------------------
Ship it!
Hi Szabi,
Thanks for this improvement, your test solution is really elegant.
Cheers,
Bogi
- Boglarka Egyed
On Dec. 6, 2016, 1:57 p.m., Szabolcs Vasas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54421/
> -----------------------------------------------------------
>
> (Updated Dec. 6, 2016, 1:57 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-3068
> https://issues.apache.org/jira/browse/SQOOP-3068
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Enhance error (tool.ImportTool: Encountered IOException running import job: java.io.IOException: Expected schema) to suggest workaround (--map-column-java)
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/avro/AvroSchemaMismatchException.java PRE-CREATION
> src/java/org/apache/sqoop/mapreduce/ParquetJob.java b077d9b
> src/java/org/apache/sqoop/tool/ImportTool.java ed951ea
> src/test/com/cloudera/sqoop/hive/TestHiveImport.java 26d087b
> src/test/org/apache/sqoop/tool/TestImportTool.java 4136e9f
> src/test/org/apache/sqoop/util/ExpectedLogMessage.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/54421/diff/
>
>
> Testing
> -------
>
> New integration test and unit test is added.
>
>
> Thanks,
>
> Szabolcs Vasas
>
>
Re: Review Request 54421: Enhance error (tool.ImportTool: Encountered
IOException running import job: java.io.IOException: Expected schema)
to suggest workaround (--map-column-java)
Posted by Erzsebet Szilagyi <er...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54421/#review159154
-----------------------------------------------------------
Fix it, then Ship it!
Hi Szabi,
Thank you for this clarifying change!
I've got only one minor question.
src/test/org/apache/sqoop/tool/TestImportTool.java (lines 73 - 80)
<https://reviews.apache.org/r/54421/#comment230121>
This is a quite big chunk of String manually edited from copy/paste by the looks of it.
Wouldn't it be better to store parts of this message in constants and build it up here from the prepared parts? I think it would make it easier to modify later.
I think the new ExpectedLogMessage class is especially useful for later enhancements of our tests.
Thank you!
Liz
- Erzsebet Szilagyi
On Dec. 6, 2016, 2:57 p.m., Szabolcs Vasas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54421/
> -----------------------------------------------------------
>
> (Updated Dec. 6, 2016, 2:57 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-3068
> https://issues.apache.org/jira/browse/SQOOP-3068
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Enhance error (tool.ImportTool: Encountered IOException running import job: java.io.IOException: Expected schema) to suggest workaround (--map-column-java)
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/avro/AvroSchemaMismatchException.java PRE-CREATION
> src/java/org/apache/sqoop/mapreduce/ParquetJob.java b077d9b
> src/java/org/apache/sqoop/tool/ImportTool.java ed951ea
> src/test/com/cloudera/sqoop/hive/TestHiveImport.java 26d087b
> src/test/org/apache/sqoop/tool/TestImportTool.java 4136e9f
> src/test/org/apache/sqoop/util/ExpectedLogMessage.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/54421/diff/
>
>
> Testing
> -------
>
> New integration test and unit test is added.
>
>
> Thanks,
>
> Szabolcs Vasas
>
>
Re: Review Request 54421: Enhance error (tool.ImportTool: Encountered
IOException running import job: java.io.IOException: Expected schema)
to suggest workaround (--map-column-java)
Posted by Anna Szonyi <sz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54421/#review160497
-----------------------------------------------------------
Thanks Szabi!
Really like the final version (mostly the ExpectedLogMessage utility, that will be super helpful :))
Thanks,
/Anna
- Anna Szonyi
On Dec. 22, 2016, 3:50 p.m., Szabolcs Vasas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54421/
> -----------------------------------------------------------
>
> (Updated Dec. 22, 2016, 3:50 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-3068
> https://issues.apache.org/jira/browse/SQOOP-3068
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Enhance error (tool.ImportTool: Encountered IOException running import job: java.io.IOException: Expected schema) to suggest workaround (--map-column-java)
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/avro/AvroSchemaMismatchException.java PRE-CREATION
> src/java/org/apache/sqoop/mapreduce/ParquetJob.java b077d9b
> src/java/org/apache/sqoop/tool/ImportTool.java ed951ea
> src/test/com/cloudera/sqoop/hive/TestHiveImport.java 26d087b
> src/test/org/apache/sqoop/tool/TestImportTool.java 4136e9f
> src/test/org/apache/sqoop/util/ExpectedLogMessage.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/54421/diff/
>
>
> Testing
> -------
>
> New integration test and unit test is added.
>
>
> Thanks,
>
> Szabolcs Vasas
>
>
Re: Review Request 54421: Enhance error (tool.ImportTool: Encountered
IOException running import job: java.io.IOException: Expected schema)
to suggest workaround (--map-column-java)
Posted by Erzsebet Szilagyi <er...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54421/#review160492
-----------------------------------------------------------
Ship it!
Hi Szabolcs,
With the changes of the changed diff, this patch looks alright to me.
Thank you for further polishing this patch!
Liz
- Erzsebet Szilagyi
On Dec. 22, 2016, 4:50 p.m., Szabolcs Vasas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54421/
> -----------------------------------------------------------
>
> (Updated Dec. 22, 2016, 4:50 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-3068
> https://issues.apache.org/jira/browse/SQOOP-3068
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Enhance error (tool.ImportTool: Encountered IOException running import job: java.io.IOException: Expected schema) to suggest workaround (--map-column-java)
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/avro/AvroSchemaMismatchException.java PRE-CREATION
> src/java/org/apache/sqoop/mapreduce/ParquetJob.java b077d9b
> src/java/org/apache/sqoop/tool/ImportTool.java ed951ea
> src/test/com/cloudera/sqoop/hive/TestHiveImport.java 26d087b
> src/test/org/apache/sqoop/tool/TestImportTool.java 4136e9f
> src/test/org/apache/sqoop/util/ExpectedLogMessage.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/54421/diff/
>
>
> Testing
> -------
>
> New integration test and unit test is added.
>
>
> Thanks,
>
> Szabolcs Vasas
>
>
Re: Review Request 54421: Enhance error (tool.ImportTool: Encountered
IOException running import job: java.io.IOException: Expected schema)
to suggest workaround (--map-column-java)
Posted by Szabolcs Vasas <va...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54421/
-----------------------------------------------------------
(Updated Dec. 22, 2016, 3:50 p.m.)
Review request for Sqoop.
Changes
-------
AvroSchemaMismatchException uses the message field of the Exception to provide more meaningful stack trace.
Bugs: SQOOP-3068
https://issues.apache.org/jira/browse/SQOOP-3068
Repository: sqoop-trunk
Description
-------
Enhance error (tool.ImportTool: Encountered IOException running import job: java.io.IOException: Expected schema) to suggest workaround (--map-column-java)
Diffs (updated)
-----
src/java/org/apache/sqoop/avro/AvroSchemaMismatchException.java PRE-CREATION
src/java/org/apache/sqoop/mapreduce/ParquetJob.java b077d9b
src/java/org/apache/sqoop/tool/ImportTool.java ed951ea
src/test/com/cloudera/sqoop/hive/TestHiveImport.java 26d087b
src/test/org/apache/sqoop/tool/TestImportTool.java 4136e9f
src/test/org/apache/sqoop/util/ExpectedLogMessage.java PRE-CREATION
Diff: https://reviews.apache.org/r/54421/diff/
Testing
-------
New integration test and unit test is added.
Thanks,
Szabolcs Vasas
Re: Review Request 54421: Enhance error (tool.ImportTool: Encountered
IOException running import job: java.io.IOException: Expected schema)
to suggest workaround (--map-column-java)
Posted by Szabolcs Vasas <va...@gmail.com>.
> On Dec. 20, 2016, 1:07 p.m., Attila Szabo wrote:
> > src/java/org/apache/sqoop/tool/ImportTool.java, lines 657-662
> > <https://reviews.apache.org/r/54421/diff/3/?file=1586430#file1586430line657>
> >
> > Hi Szabi,
> >
> > Shouldn't this be in the AvroSchemaMismatchException? IMHO it would be better from encapsulation POV, and also if the exception would be able to produce the full and proper message by itself, it would be much more useable compared to the current implementation.
> >
> > Do you agree?
Hi Attila,
Thank you for reviewing my patch! I created the AvroSchemaMismatchException to be a more general exception class and buildAvroSchemaMismatchMessage creates a message when a (hive) import failed. I can add this message building logic to AvroSchemaMismatchException but it would be less reusable then and it would be harder to handle the case when hive import was executed and an additional tip has to be printed.
- Szabolcs
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54421/#review159709
-----------------------------------------------------------
On Dec. 15, 2016, 2:42 p.m., Szabolcs Vasas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54421/
> -----------------------------------------------------------
>
> (Updated Dec. 15, 2016, 2:42 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-3068
> https://issues.apache.org/jira/browse/SQOOP-3068
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Enhance error (tool.ImportTool: Encountered IOException running import job: java.io.IOException: Expected schema) to suggest workaround (--map-column-java)
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/avro/AvroSchemaMismatchException.java PRE-CREATION
> src/java/org/apache/sqoop/mapreduce/ParquetJob.java b077d9b
> src/java/org/apache/sqoop/tool/ImportTool.java ed951ea
> src/test/com/cloudera/sqoop/hive/TestHiveImport.java 26d087b
> src/test/org/apache/sqoop/tool/TestImportTool.java 4136e9f
> src/test/org/apache/sqoop/util/ExpectedLogMessage.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/54421/diff/
>
>
> Testing
> -------
>
> New integration test and unit test is added.
>
>
> Thanks,
>
> Szabolcs Vasas
>
>
Re: Review Request 54421: Enhance error (tool.ImportTool: Encountered
IOException running import job: java.io.IOException: Expected schema)
to suggest workaround (--map-column-java)
Posted by Attila Szabo <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54421/#review159709
-----------------------------------------------------------
Fix it, then Ship it!
Hi Szabi,
This change looks awesome (especially the JUNIT rule for checking console/log output).
Although I've found some encapsulation/usability issues/suggestions.
Would you please considering them, and fixing or sharing your thoughts around?
Many thanks,
Attila
src/java/org/apache/sqoop/avro/AvroSchemaMismatchException.java (lines 28 - 55)
<https://reviews.apache.org/r/54421/#comment230739>
Hey Szabi,
Would you please consider adding a meaningful toString() method to this exception?
Without that I think it would not be as useful as it could be.
What do you think?
src/java/org/apache/sqoop/tool/ImportTool.java (lines 657 - 662)
<https://reviews.apache.org/r/54421/#comment230740>
Hi Szabi,
Shouldn't this be in the AvroSchemaMismatchException? IMHO it would be better from encapsulation POV, and also if the exception would be able to produce the full and proper message by itself, it would be much more useable compared to the current implementation.
Do you agree?
- Attila Szabo
On Dec. 15, 2016, 2:42 p.m., Szabolcs Vasas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54421/
> -----------------------------------------------------------
>
> (Updated Dec. 15, 2016, 2:42 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-3068
> https://issues.apache.org/jira/browse/SQOOP-3068
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Enhance error (tool.ImportTool: Encountered IOException running import job: java.io.IOException: Expected schema) to suggest workaround (--map-column-java)
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/avro/AvroSchemaMismatchException.java PRE-CREATION
> src/java/org/apache/sqoop/mapreduce/ParquetJob.java b077d9b
> src/java/org/apache/sqoop/tool/ImportTool.java ed951ea
> src/test/com/cloudera/sqoop/hive/TestHiveImport.java 26d087b
> src/test/org/apache/sqoop/tool/TestImportTool.java 4136e9f
> src/test/org/apache/sqoop/util/ExpectedLogMessage.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/54421/diff/
>
>
> Testing
> -------
>
> New integration test and unit test is added.
>
>
> Thanks,
>
> Szabolcs Vasas
>
>
Re: Review Request 54421: Enhance error (tool.ImportTool: Encountered
IOException running import job: java.io.IOException: Expected schema)
to suggest workaround (--map-column-java)
Posted by Szabolcs Vasas <va...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54421/
-----------------------------------------------------------
(Updated Dec. 15, 2016, 2:42 p.m.)
Review request for Sqoop.
Bugs: SQOOP-3068
https://issues.apache.org/jira/browse/SQOOP-3068
Repository: sqoop-trunk
Description
-------
Enhance error (tool.ImportTool: Encountered IOException running import job: java.io.IOException: Expected schema) to suggest workaround (--map-column-java)
Diffs (updated)
-----
src/java/org/apache/sqoop/avro/AvroSchemaMismatchException.java PRE-CREATION
src/java/org/apache/sqoop/mapreduce/ParquetJob.java b077d9b
src/java/org/apache/sqoop/tool/ImportTool.java ed951ea
src/test/com/cloudera/sqoop/hive/TestHiveImport.java 26d087b
src/test/org/apache/sqoop/tool/TestImportTool.java 4136e9f
src/test/org/apache/sqoop/util/ExpectedLogMessage.java PRE-CREATION
Diff: https://reviews.apache.org/r/54421/diff/
Testing
-------
New integration test and unit test is added.
Thanks,
Szabolcs Vasas
Re: Review Request 54421: Enhance error (tool.ImportTool: Encountered
IOException running import job: java.io.IOException: Expected schema)
to suggest workaround (--map-column-java)
Posted by Szabolcs Vasas <va...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54421/
-----------------------------------------------------------
(Updated Dec. 6, 2016, 1:57 p.m.)
Review request for Sqoop.
Bugs: SQOOP-3068
https://issues.apache.org/jira/browse/SQOOP-3068
Repository: sqoop-trunk
Description
-------
Enhance error (tool.ImportTool: Encountered IOException running import job: java.io.IOException: Expected schema) to suggest workaround (--map-column-java)
Diffs (updated)
-----
src/java/org/apache/sqoop/avro/AvroSchemaMismatchException.java PRE-CREATION
src/java/org/apache/sqoop/mapreduce/ParquetJob.java b077d9b
src/java/org/apache/sqoop/tool/ImportTool.java ed951ea
src/test/com/cloudera/sqoop/hive/TestHiveImport.java 26d087b
src/test/org/apache/sqoop/tool/TestImportTool.java 4136e9f
src/test/org/apache/sqoop/util/ExpectedLogMessage.java PRE-CREATION
Diff: https://reviews.apache.org/r/54421/diff/
Testing
-------
New integration test and unit test is added.
Thanks,
Szabolcs Vasas