----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62459/#review186052 -----------------------------------------------------------
tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 155 (patched) <https://reviews.apache.org/r/62459/#comment262433> Synchronized? tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 168 (patched) <https://reviews.apache.org/r/62459/#comment262435> Misplaced brace tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 238 (patched) <https://reviews.apache.org/r/62459/#comment262436> Do we need Integer instead of int? tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 250 (patched) <https://reviews.apache.org/r/62459/#comment262437> Do we need Integer instead of int? tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 258 (patched) <https://reviews.apache.org/r/62459/#comment262438> Do we need Integer instead of int? tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 297 (patched) <https://reviews.apache.org/r/62459/#comment262439> We shouldn't just print the exception IMO. At least we log something, re-throw it, something that is more appropriate. tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 318 (patched) <https://reviews.apache.org/r/62459/#comment262440> See my comment above about exception handling tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 337 (patched) <https://reviews.apache.org/r/62459/#comment262441> See my comment above about exception handling tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 356 (patched) <https://reviews.apache.org/r/62459/#comment262442> See my comment above about exception handling tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 407 (patched) <https://reviews.apache.org/r/62459/#comment262443> See my comment above about exception handling tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 463 (patched) <https://reviews.apache.org/r/62459/#comment262444> See my comment above about exception handling tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 483 (patched) <https://reviews.apache.org/r/62459/#comment262445> See my comment above about exception handling tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 508 (patched) <https://reviews.apache.org/r/62459/#comment262446> See my comment above about exception handling tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 523 (patched) <https://reviews.apache.org/r/62459/#comment262447> See my comment above about exception handling tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 538 (patched) <https://reviews.apache.org/r/62459/#comment262448> See my comment above about exception handling tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 548 (patched) <https://reviews.apache.org/r/62459/#comment262457> There are different places in the code where the job type is checked. I believe these should be some mini helper methods and invoke them if necessary to determine the type of a job. Something like private boolean isWorkflow(String id) { return id.endsWith("-W"); } tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 578 (patched) <https://reviews.apache.org/r/62459/#comment262449> Besides printing the stack trace, add some error message too tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 677 (patched) <https://reviews.apache.org/r/62459/#comment262450> See my comment above about exception handling tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 685 (patched) <https://reviews.apache.org/r/62459/#comment262456> Extract default encoding to a final static variable tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 788 (patched) <https://reviews.apache.org/r/62459/#comment262451> See my comment above about exception handling tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 837 (patched) <https://reviews.apache.org/r/62459/#comment262452> See my comment above about exception handling tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 872 (patched) <https://reviews.apache.org/r/62459/#comment262454> Extract default encoding to a final static variable tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 878 (patched) <https://reviews.apache.org/r/62459/#comment262453> See my comment above about exception handling tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java Lines 883 (patched) <https://reviews.apache.org/r/62459/#comment262455> Extract default encoding to a final static variable - Peter Bacsko On szept. 22, 2017, 12:50 du, Attila Sasvari wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62459/ > ----------------------------------------------------------- > > (Updated szept. 22, 2017, 12:50 du) > > > Review request for oozie. > > > Repository: oozie-git > > > Description > ------- > > A diagnostic tool that collects a bunch of job and other information from > Oozie in a zip file. > > > Diffs > ----- > > docs/src/site/twiki/DG_CommandLineTool.twiki > d4047671876dcc3279a2ec379bc1d003f5e6f1aa > pom.xml efccc346932514ada578a3462eb3c3cfe519a323 > tools/pom.xml 7306a14e7b237977be00f8fe28e34573540fd508 > tools/src/main/bin/oozie-diag-bundle-collector.sh PRE-CREATION > tools/src/main/java/org/apache/oozie/tools/OozieDiagBundleCollector.java > PRE-CREATION > > tools/src/test/java/org/apache/oozie/tools/TestOozieDiagBundleCollector.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/62459/diff/3/ > > > Testing > ------- > > - new unit tests: TestOozieDiagBundleCollector > - started Oozie with a pseudo hadoop cluster, submitted a couple workflows, > and executed the following commands: > -- ``bin/oozie-diag-bundle-collector.sh`` (usage info printed), > -- ``bin/oozie-diag-bundle-collector.sh -numworkflows 2000 -oozie > http://localhost:11000/oozie -output /tmp``, > -- ``bin/oozie-diag-bundle-collector.sh -jobs > 0000001-170918144116149-oozie-asas-W -oozie http://localhost:11000/oozie > -output .`` (verified zip the tool generated). > > > Thanks, > > Attila Sasvari > >
