----------------------------------------------------------- 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 > >
