----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66930/#review205638 -----------------------------------------------------------
tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Line 226 (original), 245 (patched) <https://reviews.apache.org/r/66930/#comment288529> Does it help to have a reference to the thread pool in every instance? We just have a single executor. I'd rather pass the executor in the methods than having this here, especially because it's not used by the class itself. tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Lines 256-259 (patched) <https://reviews.apache.org/r/66930/#comment288528> To me this looks weird. The class is named "configuration", which is telling me that it holds data. But it doesn't just hold data, it also performs an FS operation. tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Lines 339 (patched) <https://reviews.apache.org/r/66930/#comment288523> Extract magic number to a static finas with good naming tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Lines 350-351 (patched) <https://reviews.apache.org/r/66930/#comment288524> Extract magic numbers to static finals with good naming tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Lines 360-362 (patched) <https://reviews.apache.org/r/66930/#comment288525> Add error message to checkArgument() tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Line 240 (original), 405 (patched) <https://reviews.apache.org/r/66930/#comment288527> If we're not interested in the actual contents of the exception, then using a boolean to indicate a failure is a better choice. tools/src/test/java/org/apache/oozie/tools/TestCopyTaskCallable.java Lines 141 (patched) <https://reviews.apache.org/r/66930/#comment288531> This should be in a finally block - Peter Bacsko On jún. 18, 2018, 10:03 de, Kinga Marton wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66930/ > ----------------------------------------------------------- > > (Updated jún. 18, 2018, 10:03 de) > > > 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/OozieSharelibFileOperations.java > PRE-CREATION > tools/src/test/java/org/apache/oozie/tools/TestBlockSizeCalculator.java > PRE-CREATION > tools/src/test/java/org/apache/oozie/tools/TestConcurrentCopyFromLocal.java > PRE-CREATION > tools/src/test/java/org/apache/oozie/tools/TestCopyTaskCallable.java > PRE-CREATION > tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java > ccad273b > > > Diff: https://reviews.apache.org/r/66930/diff/6/ > > > Testing > ------- > > Tested manually > > > Thanks, > > Kinga Marton > >
