----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66930/#review202829 -----------------------------------------------------------
tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Lines 297 (patched) <https://reviews.apache.org/r/66930/#comment284820> A separate unit test class is needed, `TestCopyTaskCallable`, now that we wrote this to be testable. tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Lines 310-316 (patched) <https://reviews.apache.org/r/66930/#comment284814> Just before setting any fields, `Preconditions#checkNotNull()` wouldn't hurt. tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Lines 334 (patched) <https://reviews.apache.org/r/66930/#comment284821> A separate unit test class is needed, `TestConcurrentCopyFromLocal`, now that we wrote this to be testable. tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Lines 346-349 (patched) <https://reviews.apache.org/r/66930/#comment284815> Just before setting any fields, `Preconditions#checkArgument()` wouldn't hurt. tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Lines 424-425 (patched) <https://reviews.apache.org/r/66930/#comment284816> I'd have a method inside `CopyTaskConfiguration` to perform that stuff. tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Lines 429 (patched) <https://reviews.apache.org/r/66930/#comment284817> We should also log something saying we didn't manage to copy a file (exact source / destination paths, and error message provided) but we still proceed. tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Lines 437 (patched) <https://reviews.apache.org/r/66930/#comment284818> We need to express why we fail. tools/src/test/java/org/apache/oozie/tools/TestBlockSizeCalculator.java Lines 24 (patched) <https://reviews.apache.org/r/66930/#comment284819> Big thumbs up :) - András Piros On May 10, 2018, 7:59 a.m., Kinga Marton wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66930/ > ----------------------------------------------------------- > > (Updated May 10, 2018, 7:59 a.m.) > > > Review request for oozie, András Piros and Peter Cseh. > > > Repository: oozie-git > > > Description > ------- > > On a busy Hadoop cluster it can happen that users cannot install properly > Oozie ShareLib. > > > Diffs > ----- > > tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java dce1c559 > tools/src/test/java/org/apache/oozie/tools/TestBlockSizeCalculator.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/66930/diff/2/ > > > Testing > ------- > > Tested manually > > > Thanks, > > Kinga Marton > >
