----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62459/#review186568 -----------------------------------------------------------
tools/src/main/bin/oozie-diag-bundle-collector.sh Lines 23-31 (patched) <https://reviews.apache.org/r/62459/#comment263125> Extract function. tools/src/main/bin/oozie-diag-bundle-collector.sh Lines 33-34 (patched) <https://reviews.apache.org/r/62459/#comment263126> Extract function. tools/src/main/bin/oozie-diag-bundle-collector.sh Lines 36 (patched) <https://reviews.apache.org/r/62459/#comment263121> `OOZIECP` or `OOZIECLASSPATH` would be a better name. tools/src/main/bin/oozie-diag-bundle-collector.sh Lines 37-45 (patched) <https://reviews.apache.org/r/62459/#comment263122> Extract function. tools/src/main/bin/oozie-diag-bundle-collector.sh Lines 48-53 (patched) <https://reviews.apache.org/r/62459/#comment263123> Extract function. tools/src/main/bin/oozie-diag-bundle-collector.sh Lines 55-58 (patched) <https://reviews.apache.org/r/62459/#comment263124> Extract function. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 51 (patched) <https://reviews.apache.org/r/62459/#comment263127> `MAX_TIMEOUT_SECONDS` would be a better name. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 60 (patched) <https://reviews.apache.org/r/62459/#comment263141> `storeWorkflowJobDetails()` might be a better name. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 67-73 (patched) <https://reviews.apache.org/r/62459/#comment263165> This is happening three times in code. Extract to one and reuse. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 69 (patched) <https://reviews.apache.org/r/62459/#comment263135> Returning here and having no `else` path seems cleaner to me. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 71-77 (patched) <https://reviews.apache.org/r/62459/#comment263142> Extract method. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 76 (patched) <https://reviews.apache.org/r/62459/#comment263136> `"Could not create resolved actions directory:"` tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 81-83 (patched) <https://reviews.apache.org/r/62459/#comment263137> Nice :) tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 88 (patched) <https://reviews.apache.org/r/62459/#comment263138> `"Got details for ... successfully"`, maybe? tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 93 (patched) <https://reviews.apache.org/r/62459/#comment263140> Maybe printing only the `Exception` message is a better idea. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 97-98 (patched) <https://reviews.apache.org/r/62459/#comment263147> Put `resolvedActionsDir` and `configEntryWriter` to fields, and you got two parameters less. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 99-119 (patched) <https://reviews.apache.org/r/62459/#comment263144> Using a `StringBuilder`, maybe? tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 118 (patched) <https://reviews.apache.org/r/62459/#comment263143> `ACTIONS` tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 120 (patched) <https://reviews.apache.org/r/62459/#comment263148> Put this just before `while`. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 124-127 (patched) <https://reviews.apache.org/r/62459/#comment263145> Substitute w/ a normal `while` loop might be more readable. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 128-149 (patched) <https://reviews.apache.org/r/62459/#comment263146> Using a `StringBuilder`, maybe? tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 158-160 (patched) <https://reviews.apache.org/r/62459/#comment263149> `persistWorkflowAction()` tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 170 (patched) <https://reviews.apache.org/r/62459/#comment263150> It could implement `Runnable`, in which case you don't need to `return null`. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 182 (patched) <https://reviews.apache.org/r/62459/#comment263171> `StandardCharsets.UTF_8.toString()` seems cleaner to me. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 189 (patched) <https://reviews.apache.org/r/62459/#comment263151> `IllegalStateException` tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 194 (patched) <https://reviews.apache.org/r/62459/#comment263152> Print only the message. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 204 (patched) <https://reviews.apache.org/r/62459/#comment263153> Firing up and shutting down an `ExecutorService` I'd put to methods / places that are called only once per JVM / component lifetime. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 214 (patched) <https://reviews.apache.org/r/62459/#comment263177> Only message. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 217 (patched) <https://reviews.apache.org/r/62459/#comment263154> Firing up and shutting down an `ExecutorService` I'd put to methods / places that are called only once per JVM / component lifetime. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 220-233 (patched) <https://reviews.apache.org/r/62459/#comment263155> Extract to another (maybe nested) class. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 235 (patched) <https://reviews.apache.org/r/62459/#comment263156> Putting `outputDir` to a field would result in one less parameter. `storeCoordinatorJobInfo()` would be a better name. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 242-249 (patched) <https://reviews.apache.org/r/62459/#comment263164> This is happening three times in code. Extract to one and reuse. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 260-264 (patched) <https://reviews.apache.org/r/62459/#comment263157> `while` loop instead. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 270 (patched) <https://reviews.apache.org/r/62459/#comment263158> Print only the message. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 274 (patched) <https://reviews.apache.org/r/62459/#comment263159> `configEntryWriter` could possibly be a field. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 276-302 (patched) <https://reviews.apache.org/r/62459/#comment263160> `StringBuffer`? tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 304-308 (patched) <https://reviews.apache.org/r/62459/#comment263161> `while` loop tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 309-324 (patched) <https://reviews.apache.org/r/62459/#comment263162> `StringBuffer`? tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 336-343 (patched) <https://reviews.apache.org/r/62459/#comment263163> This is happening three times in code. Extract to one and reuse. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 361 (patched) <https://reviews.apache.org/r/62459/#comment263166> Print only the message. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 365 (patched) <https://reviews.apache.org/r/62459/#comment263168> `configEntryWriter` as a field tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 366-380 (patched) <https://reviews.apache.org/r/62459/#comment263167> Maybe `StringBuilder`? tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 383 (patched) <https://reviews.apache.org/r/62459/#comment263169> `storeCommonDetails` tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 399 (patched) <https://reviews.apache.org/r/62459/#comment263170> Print only the message. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 405 (patched) <https://reviews.apache.org/r/62459/#comment263172> `StandardCharsets.UTF_8.toString()` seems cleaner to me. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 428 (patched) <https://reviews.apache.org/r/62459/#comment263174> `storeLastWorkflows()` tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 440 (patched) <https://reviews.apache.org/r/62459/#comment263175> Only message. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 456 (patched) <https://reviews.apache.org/r/62459/#comment263176> Only message. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 472 (patched) <https://reviews.apache.org/r/62459/#comment263178> Only message. tools/src/main/java/org/apache/oozie/tools/diag/AppInfoCollector.java Lines 492-502 (patched) <https://reviews.apache.org/r/62459/#comment263134> Would extract to `ParentIdChecker`. Actually, sometimes the portion before `@` has to be checked, though. tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java Lines 37 (patched) <https://reviews.apache.org/r/62459/#comment263129> Please remove newline. tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java Lines 53 (patched) <https://reviews.apache.org/r/62459/#comment263190> `setupArgParseOptions` tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java Lines 54-57 (patched) <https://reviews.apache.org/r/62459/#comment263191> `createAndAddOption()` tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java Lines 59-63 (patched) <https://reviews.apache.org/r/62459/#comment263192> `createAndAddOption()` tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java Lines 65-69 (patched) <https://reviews.apache.org/r/62459/#comment263193> `createAndAddOption()` tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java Lines 71-75 (patched) <https://reviews.apache.org/r/62459/#comment263194> `createAndAddOption()` tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java Lines 77-82 (patched) <https://reviews.apache.org/r/62459/#comment263195> `createAndAddOption()` tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java Lines 84-88 (patched) <https://reviews.apache.org/r/62459/#comment263196> `createAndAddOption()` tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java Lines 90-93 (patched) <https://reviews.apache.org/r/62459/#comment263197> `createAndAddOption()` tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java Lines 96-102 (patched) <https://reviews.apache.org/r/62459/#comment263198> Covered by `createAndAddOption()` calls. tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java Lines 106 (patched) <https://reviews.apache.org/r/62459/#comment263199> `ensureOutputDir()` tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java Lines 118-120 (patched) <https://reviews.apache.org/r/62459/#comment263200> `int parseAndCheck()` tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java Lines 129-132 (patched) <https://reviews.apache.org/r/62459/#comment263201> `int parseAndCheck()` tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java Lines 136-139 (patched) <https://reviews.apache.org/r/62459/#comment263202> `int parseAndCheck()` tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java Lines 143-146 (patched) <https://reviews.apache.org/r/62459/#comment263203> `int parseAndCheck()` tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java Lines 168-173 (patched) <https://reviews.apache.org/r/62459/#comment263204> `handleParseError()` tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java Lines 40-41 (patched) <https://reviews.apache.org/r/62459/#comment263206> `Locale.getDefault()`? Is this (and need to be) thread safe? tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java Lines 43-45 (patched) <https://reviews.apache.org/r/62459/#comment263208> `int` tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java Lines 47 (patched) <https://reviews.apache.org/r/62459/#comment263210> `int` tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java Lines 95 (patched) <https://reviews.apache.org/r/62459/#comment263408> `collectAndStoreDiagInfo()` tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java Lines 102-108 (patched) <https://reviews.apache.org/r/62459/#comment263407> `collectAndStoreServerInfo()` tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java Lines 110-112 (patched) <https://reviews.apache.org/r/62459/#comment263409> `collectAndStoreMetricsInfo()` tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java Lines 114-118 (patched) <https://reviews.apache.org/r/62459/#comment263410> `collectAndStoreAppInfo()` tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java Lines 121 (patched) <https://reviews.apache.org/r/62459/#comment263212> `checkOozieConnection()` tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java Lines 136-142 (patched) <https://reviews.apache.org/r/62459/#comment263213> `initServices()` tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java Lines 149 (patched) <https://reviews.apache.org/r/62459/#comment263179> Only message. tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java Lines 39 (patched) <https://reviews.apache.org/r/62459/#comment263173> `StandardCharsets.UTF_8.toString()` seems cleaner to me. tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java Lines 41 (patched) <https://reviews.apache.org/r/62459/#comment263219> Nice :) tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java Lines 49 (patched) <https://reviews.apache.org/r/62459/#comment263412> `nullPlaceholder` tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java Lines 53 (patched) <https://reviews.apache.org/r/62459/#comment263222> `writeLong()` tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java Lines 57 (patched) <https://reviews.apache.org/r/62459/#comment263223> `writeInt()` tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java Lines 61 (patched) <https://reviews.apache.org/r/62459/#comment263224> `writeString()` tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java Lines 64 (patched) <https://reviews.apache.org/r/62459/#comment263226> Extract variable `valuePlaceholder`. tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java Lines 66 (patched) <https://reviews.apache.org/r/62459/#comment263227> Extract variable `nullPlaceholder`. tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java Lines 68 (patched) <https://reviews.apache.org/r/62459/#comment263228> Extract variable `emptyPlaceholder`. tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java Lines 70 (patched) <https://reviews.apache.org/r/62459/#comment263229> Extract variable `newline`. tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java Lines 83 (patched) <https://reviews.apache.org/r/62459/#comment263180> Only message. tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java Lines 85 (patched) <https://reviews.apache.org/r/62459/#comment263230> What about try-with-resources? tools/src/main/java/org/apache/oozie/tools/diag/Client.java Lines 33 (patched) <https://reviews.apache.org/r/62459/#comment263130> `DiagnosticsClient` would be a better name. tools/src/main/java/org/apache/oozie/tools/diag/Client.java Lines 40 (patched) <https://reviews.apache.org/r/62459/#comment263133> `createRetryingConnection()` would be a better name. tools/src/main/java/org/apache/oozie/tools/diag/Client.java Lines 42-43 (patched) <https://reviews.apache.org/r/62459/#comment263131> What about try-with-resources? tools/src/main/java/org/apache/oozie/tools/diag/Client.java Lines 48-49 (patched) <https://reviews.apache.org/r/62459/#comment263132> What about try-with-resources? tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java Lines 37 (patched) <https://reviews.apache.org/r/62459/#comment263232> `collectAndStore()` tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java Lines 51-57 (patched) <https://reviews.apache.org/r/62459/#comment263237> `writeSection()` tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java Lines 58-64 (patched) <https://reviews.apache.org/r/62459/#comment263238> `writeSection()` tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java Lines 65-74 (patched) <https://reviews.apache.org/r/62459/#comment263239> `writeMultilineSection()` tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java Lines 75-84 (patched) <https://reviews.apache.org/r/62459/#comment263240> `writeMultilineSection()` tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java Lines 89 (patched) <https://reviews.apache.org/r/62459/#comment263187> Only message. tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java Lines 93 (patched) <https://reviews.apache.org/r/62459/#comment263242> `collectAndStoreInstrumentationInfo()` tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java Lines 106-112 (patched) <https://reviews.apache.org/r/62459/#comment263243> `writeSection()` tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java Lines 113-119 (patched) <https://reviews.apache.org/r/62459/#comment263244> `writeSection()` tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java Lines 120-126 (patched) <https://reviews.apache.org/r/62459/#comment263245> `writeSection()` tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java Lines 127-136 (patched) <https://reviews.apache.org/r/62459/#comment263246> `writeMultilineSection()` tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java Lines 141 (patched) <https://reviews.apache.org/r/62459/#comment263189> Only message. tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java Lines 40 (patched) <https://reviews.apache.org/r/62459/#comment263248> `collectAndStoreShareLibInfo()` tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java Lines 49-50 (patched) <https://reviews.apache.org/r/62459/#comment263247> `while` might be better readable tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java Lines 59 (patched) <https://reviews.apache.org/r/62459/#comment263181> Only message. tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java Lines 63 (patched) <https://reviews.apache.org/r/62459/#comment263249> `collectAndStoreJavaSystemProperties()` tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java Lines 68-75 (patched) <https://reviews.apache.org/r/62459/#comment263252> `storeOneLineEntries()` tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java Lines 79 (patched) <https://reviews.apache.org/r/62459/#comment263182> Only message. tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java Lines 83 (patched) <https://reviews.apache.org/r/62459/#comment263250> `collectAndStoreOsEnv()` tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java Lines 88-94 (patched) <https://reviews.apache.org/r/62459/#comment263253> `storeOneLineEntries()` tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java Lines 99 (patched) <https://reviews.apache.org/r/62459/#comment263188> Only message. tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java Lines 103 (patched) <https://reviews.apache.org/r/62459/#comment263254> `collectAndStoreServerConfiguration()` tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java Lines 120 (patched) <https://reviews.apache.org/r/62459/#comment263183> Only message. tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java Lines 124 (patched) <https://reviews.apache.org/r/62459/#comment263255> `collectAndStoreThreadDump()` tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java Lines 131 (patched) <https://reviews.apache.org/r/62459/#comment263184> Only message. tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java Lines 135 (patched) <https://reviews.apache.org/r/62459/#comment263256> `collectAndStoreCallableQueueDump()` tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java Lines 138 (patched) <https://reviews.apache.org/r/62459/#comment263258> `dumpLines` tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java Lines 143 (patched) <https://reviews.apache.org/r/62459/#comment263257> `dumpLine` tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java Lines 152 (patched) <https://reviews.apache.org/r/62459/#comment263185> Only message. tools/src/test/java/org/apache/oozie/tools/diag/TestAppInfoCollector.java Lines 53-66 (patched) <https://reviews.apache.org/r/62459/#comment263259> `setupBasicFields()` tools/src/test/java/org/apache/oozie/tools/diag/TestAppInfoCollector.java Lines 68-76 (patched) <https://reviews.apache.org/r/62459/#comment263260> `setupDateFields()` tools/src/test/java/org/apache/oozie/tools/diag/TestAppInfoCollector.java Lines 78-101 (patched) <https://reviews.apache.org/r/62459/#comment263262> `setupActions()` tools/src/test/java/org/apache/oozie/tools/diag/TestAppInfoCollector.java Lines 115-118 (patched) <https://reviews.apache.org/r/62459/#comment263265> `setupBasicCoordinatorFields()` tools/src/test/java/org/apache/oozie/tools/diag/TestAppInfoCollector.java Lines 120-124 (patched) <https://reviews.apache.org/r/62459/#comment263266> `setupCoordinatorStatus()` tools/src/test/java/org/apache/oozie/tools/diag/TestAppInfoCollector.java Lines 138-141 (patched) <https://reviews.apache.org/r/62459/#comment263413> `setupBundle()` tools/src/test/java/org/apache/oozie/tools/diag/TestArgParser.java Lines 38 (patched) <https://reviews.apache.org/r/62459/#comment263414> `originalSecurityManager`, and not `static` tools/src/test/java/org/apache/oozie/tools/diag/TestArgParser.java Lines 45 (patched) <https://reviews.apache.org/r/62459/#comment263415> `interceptSystemSecurityManager()` tools/src/test/java/org/apache/oozie/tools/diag/TestArgParser.java Lines 65-66 (patched) <https://reviews.apache.org/r/62459/#comment263416> `interceptSystemStreams()` When do we plan setting back the original ones? tools/src/test/java/org/apache/oozie/tools/diag/TestMetricsCollector.java Lines 115 (patched) <https://reviews.apache.org/r/62459/#comment263418> woo-hoo :D tools/src/test/java/org/apache/oozie/tools/diag/TestMetricsCollector.java Lines 118 (patched) <https://reviews.apache.org/r/62459/#comment263417> Shadowing field `testOut`. tools/src/test/java/org/apache/oozie/tools/diag/TestMetricsCollector.java Lines 122-127 (patched) <https://reviews.apache.org/r/62459/#comment263419> Guava's `CharStreams#toString()` to the rescue. tools/src/test/java/org/apache/oozie/tools/diag/TestMetricsCollector.java Lines 216-221 (patched) <https://reviews.apache.org/r/62459/#comment263420> Guava's `CharStreams#toString()` to the rescue. tools/src/test/java/org/apache/oozie/tools/diag/TestMetricsCollector.java Lines 232 (patched) <https://reviews.apache.org/r/62459/#comment263186> Only message. tools/src/test/java/org/apache/oozie/tools/diag/TestServerInfoCollector.java Lines 102-106 (patched) <https://reviews.apache.org/r/62459/#comment263421> Guava's `CharStreams#toString()` to the rescue. - András Piros On Sept. 27, 2017, 2:12 p.m., Attila Sasvari wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62459/ > ----------------------------------------------------------- > > (Updated Sept. 27, 2017, 2:12 p.m.) > > > 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/diag/AppInfoCollector.java > PRE-CREATION > tools/src/main/java/org/apache/oozie/tools/diag/ArgParser.java PRE-CREATION > tools/src/main/java/org/apache/oozie/tools/diag/BundleCollectorDriver.java > PRE-CREATION > tools/src/main/java/org/apache/oozie/tools/diag/BundleCompressor.java > PRE-CREATION > tools/src/main/java/org/apache/oozie/tools/diag/BundleEntryWriter.java > PRE-CREATION > tools/src/main/java/org/apache/oozie/tools/diag/Client.java PRE-CREATION > tools/src/main/java/org/apache/oozie/tools/diag/MetricsCollector.java > PRE-CREATION > tools/src/main/java/org/apache/oozie/tools/diag/ServerInfoCollector.java > PRE-CREATION > tools/src/test/java/org/apache/oozie/tools/diag/TestAppInfoCollector.java > PRE-CREATION > tools/src/test/java/org/apache/oozie/tools/diag/TestArgParser.java > PRE-CREATION > tools/src/test/java/org/apache/oozie/tools/diag/TestMetricsCollector.java > PRE-CREATION > > tools/src/test/java/org/apache/oozie/tools/diag/TestServerInfoCollector.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/62459/diff/7/ > > > 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 > >
