----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64417/#review193256 -----------------------------------------------------------
Ship it! Hi Fero, Thanks for your contribution and improving this patch! Unit and 3rd party tests passed with your change. Thanks, Bogi - Boglarka Egyed On Dec. 8, 2017, 3:55 p.m., Fero Szabo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64417/ > ----------------------------------------------------------- > > (Updated Dec. 8, 2017, 3:55 p.m.) > > > Review request for Sqoop, Boglarka Egyed and Szabolcs Vasas. > > > Bugs: SQOOP-3153 > https://issues.apache.org/jira/browse/SQOOP-3153 > > > Repository: sqoop-trunk > > > Description > ------- > > The errormessages are generated by the hasUnrecognizedArgs funciton, which is > located in the base class of every Tool: BaseSqoopTool. > > I overrided the hasUnrecognizedArgs function, in the subclass (i.e. the > ExportTool class), this calls the original function first and then checks > whether the file formats were specified and prints an additional hint / error > message. > > Example output: > 2017-12-07 16:02:42,640 ERROR [main] tool.BaseSqoopTool > (BaseSqoopTool.java:hasUnrecognizedArgs(335)) - Error parsing arguments for > export: > 2017-12-07 16:02:42,643 ERROR [main] tool.BaseSqoopTool > (BaseSqoopTool.java:hasUnrecognizedArgs(338)) - Unrecognized argument: > --as-parquetfile > 2017-12-07 16:02:42,643 ERROR [main] tool.ExportTool > (ExportTool.java:hasUnrecognizedArgs(405)) - Please note that the export tool > detects the file format automatically and does not support it as an argument: > --as-parquetfile > > Also, corrected a typo in the function javadoc. > > Concerns: > - Please let me know if a different message would provide more clarity. > - The intention was to implement the decorator pattern here, please feel free > to point it out if a different, simpler design would make more sense. > > > Diffs > ----- > > src/java/org/apache/sqoop/tool/BaseSqoopTool.java 6a4dcb09 > src/java/org/apache/sqoop/tool/ExportTool.java cd6cdf32 > > > Diff: https://reviews.apache.org/r/64417/diff/4/ > > > Testing > ------- > > The following tests passed: > - Unit tests > - 3rd party integration tests > > I haven't added a new test case since testHCatExportWithParquetFile in the > TestHCatalogBasic class already covers the code path and didn't require > modification. > > > Thanks, > > Fero Szabo > >
