----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59811/#review177230 -----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java Line 122 (original), 123 (patched) <https://reviews.apache.org/r/59811/#comment250772> I really like the "fail fast" idea! Can I suggest a minor reordering of a few lines that might be more readable? (The basic idea is just to move the lines into a narrower scope when possible). ``` List<Object> results = (List<Object>) estimateLogSize(args, server).getResult(); if (!results.isEmpty()) { if (results.get(0) instanceof Long) { long estimatedSize = (Long) results.get(0); logger.info("Received estimated export size from member {}: {}", server.getId(), estimatedSize); totalEstimatedExportSize += estimatedSize; } else if (results.get(0) instanceof ManagementException) { ManagementException exception = (ManagementException) results.get(0); return ResultBuilder.createUserErrorResult(exception.getMessage()); } } ``` - Jared Stewart On June 7, 2017, 6:35 p.m., Ken Howe wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59811/ > ----------------------------------------------------------- > > (Updated June 7, 2017, 6:35 p.m.) > > > Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Kirk Lund, > and Patrick Rhomberg. > > > Repository: geode > > > Description > ------- > > GEODE-2420: add file-size-limit param to the ExportLogsController > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java > 0ff780cbf66937d8ececfb3a2d0789ee485b9b62 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java > 57355c0efae4c6da9470267f95e27e59aa4d8b2c > > geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java > 9f68d3a5eaadbe8f1bd95ec8df85ed1f65aa67ce > > geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java > a369c6e1ffb330715fbde2cd69d023ed36f133ad > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java > 16549e70bbebf4390bb73a481274e92ca6cad035 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java > 8609b3aaf0a0eb1ba903bd39c64103f9510a6a78 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java > 09ee08dd6af29b9a418ef7499defc4980da787ed > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsStatsDUnitTest.java > 44a036298e0991c880fc552596d296e104b97ca1 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java > 4e1dac013d239437829bc52dc70689c4ba15dc58 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionTest.java > cc5e7d5256741ad0a48ff87c7f989a18b90f7f03 > > > Diff: https://reviews.apache.org/r/59811/diff/2/ > > > Testing > ------- > > 6/7/17: re-started precheckin > > Precheckin started > > > Thanks, > > Ken Howe > >
