----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67834/#review205796 -----------------------------------------------------------
docs/src/site/twiki/AG_Install.twiki Lines 60 (patched) <https://reviews.apache.org/r/67834/#comment288693> Typo: comma. docs/src/site/twiki/AG_Install.twiki Lines 61 (patched) <https://reviews.apache.org/r/67834/#comment288695> Would be better to use mixed separators as per functionality: ``` sharelib-name-1=path-name-1,path-name-2:sharelib-name-2=path-name-1,path-name-2 ``` tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Lines 74-86 (patched) <https://reviews.apache.org/r/67834/#comment288694> Extracting `System.lineSeparator()` to a constant field would increase readability. tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Lines 88 (patched) <https://reviews.apache.org/r/67834/#comment288696> Problem is we can have ```hdfs://my/jar.jar#myjar.jar``` inside sharelib paths. Best is to go with e.g. `;` that cannot be present as HDFS or local file. tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Lines 264 (patched) <https://reviews.apache.org/r/67834/#comment288698> Shouldn't it be `System.out`? tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java Lines 282-294 (patched) <https://reviews.apache.org/r/67834/#comment288699> A log message about how parallel we are running would be nice. tools/src/test/java/org/apache/oozie/tools/OozieSharelibFileOperations.java Lines 42-48 (original), 43-51 (patched) <https://reviews.apache.org/r/67834/#comment288701> Could be extracted to a nested `static class`, and tested separately. tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java Lines 181-182 (original), 112-113 (patched) <https://reviews.apache.org/r/67834/#comment288702> Multiple testing scenarios missing: * empty value to a key present * multiple values to a key * value without a key Please use a separate test file. tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java Lines 116-118 (patched) <https://reviews.apache.org/r/67834/#comment288703> `message` missing. tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java Lines 143 (patched) <https://reviews.apache.org/r/67834/#comment288704> `message` missing. tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java Lines 152 (patched) <https://reviews.apache.org/r/67834/#comment288705> `message` missing. tools/src/test/java/org/apache/oozie/tools/diag/IntegrationTestOozieSharelibCLI.java Lines 84 (patched) <https://reviews.apache.org/r/67834/#comment288706> `message` missing. tools/src/test/java/org/apache/oozie/tools/diag/IntegrationTestOozieSharelibCLI.java Lines 104 (patched) <https://reviews.apache.org/r/67834/#comment288707> `message` missing. tools/src/test/java/org/apache/oozie/tools/diag/IntegrationTestOozieSharelibCLI.java Lines 130 (patched) <https://reviews.apache.org/r/67834/#comment288708> `message` missing. tools/src/test/java/org/apache/oozie/tools/diag/IntegrationTestOozieSharelibCLI.java Lines 147 (patched) <https://reviews.apache.org/r/67834/#comment288709> Nice `message` :) - András Piros On July 6, 2018, 8:36 a.m., Kinga Marton wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67834/ > ----------------------------------------------------------- > > (Updated July 6, 2018, 8:36 a.m.) > > > Review request for oozie, András Piros, Peter Cseh, and Peter Bacsko. > > > Repository: oozie-git > > > Description > ------- > > Right now sharelib can be created via sharelib create -fs FS_URI -locallib > SHARED_LIBRARY where the SHARED_LIBRARY can be a tarbal or a folder. > It would be nice to have the possibility to define additional folders to be > uploaded into the sharelib, so the users don't have to copy or link the files > together on their machine. > The syntax could be something like -additional-lib > sharelibName=/path/to/source/;/path/to/some/file,sharelibName2=/path/to/some/folder > > > Diffs > ----- > > docs/src/site/twiki/AG_Install.twiki 46363a3b3 > tools/src/main/java/org/apache/oozie/tools/OozieSharelibCLI.java 75e932c02 > tools/src/test/java/org/apache/oozie/tools/OozieSharelibFileOperations.java > d344300ed > tools/src/test/java/org/apache/oozie/tools/TestConcurrentCopyFromLocal.java > d77eba69d > tools/src/test/java/org/apache/oozie/tools/TestCopyTaskCallable.java > bce0433c6 > tools/src/test/java/org/apache/oozie/tools/TestOozieSharelibCLI.java > 5929e5ca0 > > tools/src/test/java/org/apache/oozie/tools/diag/IntegrationTestOozieSharelibCLI.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/67834/diff/4/ > > > Testing > ------- > > Tested manually + added integration test > > > Thanks, > > Kinga Marton > >
