> On May 20, 2015, 7 a.m., Abraham Elmahrek wrote: > > Have a few nits. Also, could you add a test or two to TestMerge.java or an > > equivalent?
I will add some unit tests. Will create another reviewboard after that. > On May 20, 2015, 7 a.m., Abraham Elmahrek wrote: > > src/java/org/apache/sqoop/mapreduce/MergeJob.java, line 169 > > <https://reviews.apache.org/r/34390/diff/1/?file=963322#file963322line169> > > > > Might want DEBUG logging to log schemas. This can get really tricky in > > practice. There is a DEBUG level logging after this exception: ``` LOG.debug("Avro files' Schema:" + oldPathSchema); ``` Is this OK? > On May 20, 2015, 7 a.m., Abraham Elmahrek wrote: > > src/java/org/apache/sqoop/mapreduce/MergeJob.java, lines 184-187 > > <https://reviews.apache.org/r/34390/diff/1/?file=963322#file963322line184> > > > > Might want to check if these are avro files. Avro has a header that > > should be grokkable: > > https://github.com/apache/avro/blob/trunk/lang/py/src/avro/datafile.py#L145. > > > > Also, I think there are sometimes 2 avro files: "*.avro" and "*.asc". The file type has been checked in `runMergeJob`: ``` FileType fileType = ExportJobBase.getFileType(jobConf, oldPath); ``` - Yibing ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34390/#review84347 ----------------------------------------------------------- On May 19, 2015, 11:50 a.m., Yibing Shi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34390/ > ----------------------------------------------------------- > > (Updated May 19, 2015, 11:50 a.m.) > > > Review request for Sqoop and Jarek Cecho. > > > Repository: sqoop-trunk > > > Description > ------- > > SQOOP-1094 Add avro support to merge tool > > > Diffs > ----- > > src/java/org/apache/sqoop/mapreduce/AvroJob.java > bb4755c880d4b70f812caf5e812135400602ee36 > src/java/org/apache/sqoop/mapreduce/MergeAvroReducer.java PRE-CREATION > src/java/org/apache/sqoop/mapreduce/MergeAvrodMapper.java PRE-CREATION > src/java/org/apache/sqoop/mapreduce/MergeJob.java > 4e2a916911e7f47838366edf46b5ba5073502453 > src/java/org/apache/sqoop/mapreduce/MergeReducer.java > cafff8ab0609cd0580ff85e49082a59ce68e7141 > src/java/org/apache/sqoop/mapreduce/MergeReducerBase.java PRE-CREATION > > Diff: https://reviews.apache.org/r/34390/diff/ > > > Testing > ------- > > > Thanks, > > Yibing Shi > >
