----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66930/#review204598 -----------------------------------------------------------
tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Lines 70-71 (patched) <https://reviews.apache.org/r/66930/#comment287176> Please check if these constants can be imported from some Hadoop class. tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Line 184 (original), 193-194 (patched) <https://reviews.apache.org/r/66930/#comment287177> Does getConf() return a Configuration object? If so, call getLong() so you don't have to call parseLong(). tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Lines 214 (patched) <https://reviews.apache.org/r/66930/#comment287178> What about using logError() ? tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Lines 294 (patched) <https://reviews.apache.org/r/66930/#comment287179> Nit: unnecessary white space after "0" tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Lines 298 (patched) <https://reviews.apache.org/r/66930/#comment287180> Ok, I know this is nitpicking, but still... :D It's better to write "(ratio + 1)", so with whitespaces. tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Line 268 (original), 439 (patched) <https://reviews.apache.org/r/66930/#comment287181> Initially we did something similar in the DB retry logic. The problem with this approach is that retryDelayInMs can grow to a large number in an unfortunate situation. We're better off cap it to a reasonable maximum like 30 secs or something. tools/src/test/java/org/apache/oozie/tools/TestBlockSizeCalculator.java Lines 40 (patched) <https://reviews.apache.org/r/66930/#comment287182> Nite: could you move this lone comma to the end of the previous line? tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLIHelper.java Lines 35 (patched) <https://reviews.apache.org/r/66930/#comment287183> Does the methods in this class have to be protected? If this is an utility class and not intended to be subclassed, we should be fine with public methods (plus declare it final). Also, if we don't want instances, we can make the constructor private. - Peter Bacsko On jún. 6, 2018, 12:34 du, Kinga Marton wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66930/ > ----------------------------------------------------------- > > (Updated jún. 6, 2018, 12:34 du) > > > 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 > 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 > tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLIHelper.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/66930/diff/5/ > > > Testing > ------- > > Tested manually > > > Thanks, > > Kinga Marton > >
