> On Feb. 23, 2017, 10:07 p.m., Kirk Lund wrote: > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommands.java, > > line 1051 > > <https://reviews.apache.org/r/56991/diff/1/?file=1645937#file1645937line1051> > > > > Most of the command functions we inherited do something like this but > > it loses the stack trace. I think we want to consider using the full > > stackTrackAsString -- example: > > > > result = > > ResultBuilder.createGemFireErrorResult(CliStrings.SHOW_LOG_ERROR + > > CliUtil.stackTraceAsString(e)); > > > > Better yet use sendException (I marked a later line where you use > > getResult().sendException() and I think that's the pattern we want to > > change all command functions to use.
the previous line does "logger.error(e.getMessage(), e)", which logs the stack trace. We cann't do sendException here since this is in a interceptor. > On Feb. 23, 2017, 10:07 p.m., Kirk Lund wrote: > > geode-core/src/main/java/org/apache/geode/management/internal/cli/util/ExportLogsRepository.java, > > line 21 > > <https://reviews.apache.org/r/56991/diff/1/?file=1645942#file1645942line21> > > > > Should this be an Interface instead of a Class? This seems wrong. At a > > minimum, I'd expect it to be abstract, but I would delete "Map exportFiles" > > since this class doesn't use it. The highest subclass that uses this var > > should define it. I believe this is still wip. - Jinmei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56991/#review166581 ----------------------------------------------------------- On Feb. 23, 2017, 6:01 p.m., Jinmei Liao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56991/ > ----------------------------------------------------------- > > (Updated Feb. 23, 2017, 6:01 p.m.) > > > Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk > Lund. > > > Repository: geode > > > Description > ------- > > I am putting this up for review for now. It's a big changeset which includes > the following JIRA tickets: basically it's using replicated region as a > communication channel to transfer logs from remote servers to the > jmx-manager/locator that initiates this command, and then gather all the logs > as a http response to send it back to the gfsh client: > > GEODE-2414: Determine a mechanism to stream a zip file from server to locator > GEODE-2415: Write a function to return a zip file for a single server > GEODE-2414: Create exportLogs region for all locators > GEODE-2418: enable gfsh to download file from http connection > > > Diffs > ----- > > geode-core/src/main/java/org/apache/geode/management/cli/CliMetaData.java > a20fba5a1b9fa185283bd7c60c4ade77345ed4ac > > geode-core/src/main/java/org/apache/geode/management/internal/cli/AbstractCliAroundInterceptor.java > de24727819856d180a69ed0021c21aa016e71712 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/CliAroundInterceptor.java > 11d74c13311923357e82318b32bf0342e156e0c6 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java > 8525b5862274cbaa98e57455869d404465851291 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandRequest.java > b0242c9e8ee656001cf76155f4e727ece07666a2 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommands.java > bbf0542b80469e11d21a2ed56f52ef9e647e91ea > > geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java > PRE-CREATION > > geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java > c536e8e99ef32043d9caf267dd563ee2d05b6e50 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategy.java > b53c53f91ed63a01bfaa1232d9e194481ae45664 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/util/ExportLogsCacheWriter.java > PRE-CREATION > > geode-core/src/main/java/org/apache/geode/management/internal/cli/util/ExportLogsRepository.java > PRE-CREATION > > geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java > PRE-CREATION > > geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogFilter.java > PRE-CREATION > > geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogLevelExtractor.java > PRE-CREATION > > geode-core/src/main/java/org/apache/geode/management/internal/cli/util/MergeLogs.java > 8d2ef4599337cb091da6f26c6034b1a71311148d > > geode-core/src/main/java/org/apache/geode/management/internal/configuration/utils/ZipUtils.java > 81161d6f3e2539ca3d09765e99b891d1522da302 > > geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/MiscellaneousCommandsController.java > ac912c82c873c443dd268f07e8874b5bd18fdd0b > > geode-core/src/main/java/org/apache/geode/management/internal/web/shell/AbstractHttpOperationInvoker.java > fa052489772e3e03eb865d17dbbcb7e227813c42 > > geode-core/src/main/java/org/apache/geode/management/internal/web/shell/RestHttpOperationInvoker.java > 82b2b1fc0d2c8952835b0101c413161d39f361dc > > geode-core/src/main/java/org/apache/geode/management/internal/web/shell/SimpleHttpOperationInvoker.java > dcda27bef61bad1d7caf1fe1a99063f99d768819 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/CommandRequestTest.java > 543623766b3422d63a1c3ceca38229144c94076b > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnit.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart1DUnitTest.java > 2fc8e844d319abcf19430cfb9cd09e2a64aab38c > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart2DUnitTest.java > d1f44d131bb6c4abe944d5bfd319ebe267b2f7b2 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart3DUnitTest.java > b6a4a5ba3bce03cae8c46442cb032ec270455ed9 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart4DUnitTest.java > b712785b74e4f65fbfedbb362a129151e34c18bb > > geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunctionIntegrationTest.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterTest.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogFilterTest.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogLevelExtractorTest.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/management/internal/cli/util/MergeLogsTest.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/management/internal/configuration/EventTestCacheWriter.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/management/internal/configuration/ZipUtilsJUnitTest.java > 1791574bc688774900ba2e609f3c80600cb5cac9 > > geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java > 2d32905d45496fe69e5dc189cd94b82ea163c3b5 > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java > 4729be3f4d9b51168422784a57ac1ec76e018e83 > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java > c3e493e1c176974ed757c71d5c195ff5964dbb57 > geode-core/src/test/java/org/apache/geode/test/dunit/rules/Member.java > 7cc1eea2e26b176b570111bccdb8914e19528ecf > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java > b5ddee666c52fe39deb015c6e2aa57e5b21e4f20 > > geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt > 8c0fb4569d0fbfa99622a18bac41281b1cc561df > > geode-core/src/test/resources/org/apache/geode/management/internal/cli/commands/golden-help-offline.properties > 694d4ffd2c01b0e99010fa425fcbeac08029843b > > geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/CommandOverHttpDUnitTest.java > d4869c2110f69275c566eb52e5d09a38ebe30589 > > geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsOverHttpDunit.java > PRE-CREATION > > geode-web/src/test/java/org/apache/geode/management/internal/web/shell/RestHttpOperationInvokerJUnitTest.java > 808c78f0f4a39501563a389e8b156452c48ffe81 > > geode-web/src/test/java/org/apache/geode/management/internal/web/shell/SimpleHttpOperationInvokerJUnitTest.java > 8afbcf683824545dff050d88ef320a260855c056 > > Diff: https://reviews.apache.org/r/56991/diff/ > > > Testing > ------- > > precheckin running > > > Thanks, > > Jinmei Liao > >
