You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@geode.apache.org by Jinmei Liao <ji...@pivotal.io> on 2017/03/02 15:40:15 UTC

Review Request 57243: GEODE-2416: Collect together artifacts from individual servers into a single zip file

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57243/
-----------------------------------------------------------

Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.


Repository: geode


Description
-------

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-2418: enable gfsh to download file from http connection
GEODE-2267: add validation to the arguments and include export stats in the command
GEODE-2267: use the config to determine where the logs and stats are
GEODE-2267: Enhance server/locator startup rules to include workingDir


Diffs
-----

  geode-assembly/build.gradle 1c95927f19888b15e771b3cbbc92f120787868c4 
  geode-assembly/src/test/java/org/apache/geode/management/internal/AgentUtilJUnitTest.java 3a29e8a435f5d5362810f5e05ea92a1260e3fbfc 
  geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java f0b6bb327f0424f2e4afa2e057d8e38bd0ef1b39 
  geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java 160f6349d94646b12e3114da14e5bff569eed5a9 
  geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java 10ce515df8c3630cbf01fc314be454d3e2adfe2c 
  geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java c7d0e7341fcf13bf34a597405d2b42b608004215 
  geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java b5472909eec8d5ca124e7bfd5c6cb71864d9bbee 
  geode-core/src/main/java/org/apache/geode/distributed/DistributedSystem.java 20c948f8f16240aef5c8eae313164693125a99ca 
  geode-core/src/main/java/org/apache/geode/internal/logging/GemFireLevel.java cffd162687573c9f7861bc2ada1076293c2e0a57 
  geode-core/src/main/java/org/apache/geode/internal/logging/InternalLogWriter.java 661fce99f01a890b727daf7dc591316d1d8982fb 
  geode-core/src/main/java/org/apache/geode/internal/logging/LogService.java 0ad7d848a05a0104ce01652b1fb628f2cff1b2d1 
  geode-core/src/main/java/org/apache/geode/internal/statistics/StatArchiveHandler.java 03b0f898ad30b6b088108fa0187b0a5a750ed6dc 
  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 8cd098d9719774c49efcf28d1d5ecdcd345687fc 
  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/functions/LogFileFunction.java 41ffeb40b4dd09ed2bf7eb4ef2cd516001927025 
  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/internal/statistics/SimpleStatSamplerIntegrationTest.java 167fa3d07802136ea68ec2f4f54e7c4716ea938a 
  geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java 83a367eb88aec85984691e651e5de0f8b479c7cb 
  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/HeadlessGfsh.java 72be0c7fa6839ff7f818cffa4ee5b436bebd2ce8 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CliCommandTestBase.java 21426d67904758aad4b1cf8b8c8966dd0fd45365 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java 05f2c0cb2115ee574666ed3da943c26e8c38c9f2 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptorJUnitTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsOnServerManagerDUnit.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportStatsDUnitTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart1DUnitTest.java cb9a8c6f74ba2a7b9799ad049a4ebe2861b9087b 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart2DUnitTest.java 2b2e524c5369950073099c3509c8f30e768607f6 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart3DUnitTest.java efef2c4abf70deeaef81285f13997542490234e3 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart4DUnitTest.java 9d64bd9ed3fec910c2b5dd4e8be50d3368c63893 
  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/ExportLogsCacheWriterUnitTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterIntegrationTest.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/MergeLogsDUnitTest.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/CacheServerMBeanShiroJUnitTest.java 3183aafd9907ecb15714c2038f2b8c3af31e6e06 
  geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java ea72cfab26f828af9a09f08755a9a7f9ed121654 
  geode-core/src/test/java/org/apache/geode/management/internal/security/DeployCommandsSecurityTest.java e89bd62b59b8218c30c8f724b4febf2b3ae7721b 
  geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java 0980c18de1b504f545355a0f7d650f46c92cc670 
  geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java 2d32905d45496fe69e5dc189cd94b82ea163c3b5 
  geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java 2ae61402d330ab6426c0bc0e0193771ce5dc9c70 
  geode-core/src/test/java/org/apache/geode/management/internal/security/MultiUserDUnitTest.java 37e7f453dab01dbcfcc26ebacaf3c27cc680a5c8 
  geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java 52afab40990ab3a18fed04fcb001ec9dbe7d045e 
  geode-core/src/test/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java e87a285d088c923326aa82ee406f543ffc5e0593 
  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/Locator.java fe26e8364798689c1ece487554d99085f3f26f23 
  geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerConfigurationRule.java 0a6396263aa18629b178d82932be014a40c134a9 
  geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java 68447201875ef31a73cfb888d346292e3f084ae8 
  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/Server.java 4aa2c69389dce4650cf476f4e8decebddfc36736 
  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/build.gradle ffab391d97792ce82fa21fecbc00afb45fe820d6 
  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/ExportLogsOverHttpDUnitTest.java PRE-CREATION 
  geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/QueryNamesOverHttpDUnitTest.java 1a19e5e5ca90c248655eb40e68aca3c7ed858770 
  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/57243/diff/1/


Testing
-------

precheckin running (hopefully last one)


Thanks,

Jinmei Liao

Re: Review Request 57243: GEODE-2416: Collect together artifacts from individual servers into a single zip file

Posted by Jinmei Liao <ji...@pivotal.io>.


> On March 2, 2017, 7:24 p.m., Kirk Lund wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java
> > Lines 308 (patched)
> > <https://reviews.apache.org/r/57243/diff/1/?file=1653690#file1653690line308>
> >
> >     You should create CliUtilTest or CliUtilIntegrationTest and write some tests for these new methods.

there are dunit tests around it. Will add more tests.


> On March 2, 2017, 7:24 p.m., Kirk Lund wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommands.java
> > Line 782 (original), 694 (patched)
> > <https://reviews.apache.org/r/57243/diff/1/?file=1653692#file1653692line821>
> >
> >     We should not modify this method in place (in MiscellaneousCommands) without extracting it and introducing unit tests.
> >     
> >     Example:
> >     
> >     1) extract this command to its own class "ExportLogsCommand"
> >     
> >     2) extract the guts of this method to another class (maybe called ExportLogs)
> >     
> >     ExportLogs becomes a simple class that does not know anything about GFSH or CLI parsing or those annotations. It just knows how to do the actual work based on arguments passed to its constructor.
> >     
> >     ExportLogsCommand becomes a simple class that uses ExportLogs within the context of GFSH and handles all of the CLI annotations.
> >     
> >     Then we can write a true mocking UnitTest for ExportLogs and test this class and its functionality in isolation from GFSH. All classes should end up being testable with a true UnitTest and classes should be refactored to follow good OOP practices (single responsibility, etc). This is not an OOP class as currently written.
> >     
> >     This method should also be broken down into nice small methods that can be tested and tests should be written for them.

I extracted the command to its own class, but left the implementation intact for now. We have test and integration tests that tests around LogExporter. Will file a JIRA ticket to make the exportlogcommand more testable.


> On March 2, 2017, 7:24 p.m., Kirk Lund wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommands.java
> > Line 833 (original), 733 (patched)
> > <https://reviews.apache.org/r/57243/diff/1/?file=1653692#file1653692line878>
> >
> >     At a minimum, I'd hoist this "Region region =" out of the for-loop and invoke it just once.

the region is destroyed by the receiver of the function (because it's a replicated region, once the region is destroyed on the server, it will be destroyed on the locator). 

We are doing this because if we leave the regions around, then then the put messages will be broadcasted to all the members. We had to choose between the network overhead or region create/destroy overhead. We chose the latter.


> On March 2, 2017, 7:24 p.m., Kirk Lund wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java
> > Lines 57 (patched)
> > <https://reviews.apache.org/r/57243/diff/1/?file=1653693#file1653693line57>
> >
> >     This class should be a Function that uses another class called LogExporter.
> >     
> >     LogExporter should be a class that follows solid OOP principles and has a new UnitTest that uses Mockito to test it in isolation.
> >     
> >     ExportLogsFunction then becomes an integration class that uses LogExporter within the context of a Function.

It does use a LogExporter.


> On March 2, 2017, 7:24 p.m., Kirk Lund wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java
> > Lines 119 (patched)
> > <https://reviews.apache.org/r/57243/diff/1/?file=1653693#file1653693line119>
> >
> >     Why is this public? Is this called from other classes?

Yes, the LogExportCommand uses it to craete the region on the locator.


> On March 2, 2017, 7:24 p.m., Kirk Lund wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java
> > Lines 142 (patched)
> > <https://reviews.apache.org/r/57243/diff/1/?file=1653693#file1653693line142>
> >
> >     Does this need to be public?

yes, the command uses it.


> On March 2, 2017, 7:24 p.m., Kirk Lund wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java
> > Lines 159 (patched)
> > <https://reviews.apache.org/r/57243/diff/1/?file=1653693#file1653693line159>
> >
> >     Can this be made private?

the command will need to create the argument.


> On March 2, 2017, 7:24 p.m., Kirk Lund wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java
> > Lines 208 (patched)
> > <https://reviews.apache.org/r/57243/diff/1/?file=1653693#file1653693line208>
> >
> >     Can this be made private?

command uese it.


- Jinmei


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57243/#review167709
-----------------------------------------------------------


On March 2, 2017, 7:26 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57243/
> -----------------------------------------------------------
> 
> (Updated March 2, 2017, 7:26 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> 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-2418: enable gfsh to download file from http connection
> GEODE-2267: add validation to the arguments and include export stats in the command
> GEODE-2267: use the config to determine where the logs and stats are
> GEODE-2267: Enhance server/locator startup rules to include workingDir
> 
> What's not done in this batch:
> 1. If you are exporting without http connection, exported zip will be left on the loator's working dir, currently we don't have a cleanup mechanism to delete them.
> 2. long running task may timeout the http connection. No test around this scenario.
> 3. No warning to the user if they are exporting a large quantity of logs.
> 4. we are exporting all the .log and .gfs files and only those files. Other file extensions are ignored.
> 5. merge-log option is ignored
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/internal/logging/GemFireLevel.java cffd162687573c9f7861bc2ada1076293c2e0a57 
>   geode-core/src/main/java/org/apache/geode/internal/logging/InternalLogWriter.java 661fce99f01a890b727daf7dc591316d1d8982fb 
>   geode-core/src/main/java/org/apache/geode/internal/logging/LogService.java 0ad7d848a05a0104ce01652b1fb628f2cff1b2d1 
>   geode-core/src/main/java/org/apache/geode/internal/statistics/StatArchiveHandler.java 03b0f898ad30b6b088108fa0187b0a5a750ed6dc 
>   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 8cd098d9719774c49efcf28d1d5ecdcd345687fc 
>   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/functions/LogFileFunction.java 41ffeb40b4dd09ed2bf7eb4ef2cd516001927025 
>   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/internal/statistics/SimpleStatSamplerIntegrationTest.java 167fa3d07802136ea68ec2f4f54e7c4716ea938a 
>   geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java 83a367eb88aec85984691e651e5de0f8b479c7cb 
>   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/HeadlessGfsh.java 76e986df2c69bfde9d7311138f8727610276876f 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CliCommandTestBase.java 21426d67904758aad4b1cf8b8c8966dd0fd45365 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptorJUnitTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsOnServerManagerDUnit.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportStatsDUnitTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart1DUnitTest.java cb9a8c6f74ba2a7b9799ad049a4ebe2861b9087b 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart2DUnitTest.java 2b2e524c5369950073099c3509c8f30e768607f6 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart3DUnitTest.java efef2c4abf70deeaef81285f13997542490234e3 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart4DUnitTest.java 9d64bd9ed3fec910c2b5dd4e8be50d3368c63893 
>   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/ExportLogsCacheWriterUnitTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterIntegrationTest.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/MergeLogsDUnitTest.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 d1750c3a5efd636c0f9f47fabe670265bf46f072 
>   geode-core/src/test/java/org/apache/geode/management/internal/security/MultiUserDUnitTest.java 37e7f453dab01dbcfcc26ebacaf3c27cc680a5c8 
>   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/LocatorServerConfigurationRule.java 0a6396263aa18629b178d82932be014a40c134a9 
>   geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java 39c13d000db80bef37563729bc17ae4bcb566153 
>   geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java 84c660cfe0ab9b7c6ba0bfc08b034ccaf17697f0 
>   geode-core/src/test/java/org/apache/geode/test/dunit/rules/Member.java 5f46da21f380a67a8e7f4855da1dfbb3118057ba 
>   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/build.gradle ffab391d97792ce82fa21fecbc00afb45fe820d6 
>   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/ExportLogsOverHttpDUnitTest.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/57243/diff/2/
> 
> 
> Testing
> -------
> 
> precheckin running (hopefully last one)
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 57243: GEODE-2416: Collect together artifacts from individual servers into a single zip file

Posted by Kirk Lund <ki...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57243/#review167709
-----------------------------------------------------------



I'm going to publish what I have so far for review. This is for page 1 of the diffs.


geode-core/src/main/java/org/apache/geode/management/internal/cli/CliAroundInterceptor.java
Lines 40 (patched)
<https://reviews.apache.org/r/57243/#comment239615>

    This is an internal class. Can we just delete methods like this instead of deprecating it?



geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java
Lines 308 (patched)
<https://reviews.apache.org/r/57243/#comment239624>

    You should create CliUtilTest or CliUtilIntegrationTest and write some tests for these new methods.



geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandRequest.java
Lines 43 (patched)
<https://reviews.apache.org/r/57243/#comment239626>

    I recommend making this final and alter the constructor:
    ```java
    this.downloadFile = metaData != null && metaData.isFileDownloadOverHttp();
    ```



geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommands.java
Line 782 (original), 694 (patched)
<https://reviews.apache.org/r/57243/#comment239635>

    We should not modify this method in place (in MiscellaneousCommands) without extracting it and introducing unit tests.
    
    Example:
    
    1) extract this command to its own class "ExportLogsCommand"
    
    2) extract the guts of this method to another class (maybe called ExportLogs)
    
    ExportLogs becomes a simple class that does not know anything about GFSH or CLI parsing or those annotations. It just knows how to do the actual work based on arguments passed to its constructor.
    
    ExportLogsCommand becomes a simple class that uses ExportLogs within the context of GFSH and handles all of the CLI annotations.
    
    Then we can write a true mocking UnitTest for ExportLogs and test this class and its functionality in isolation from GFSH. All classes should end up being testable with a true UnitTest and classes should be refactored to follow good OOP practices (single responsibility, etc). This is not an OOP class as currently written.
    
    This method should also be broken down into nice small methods that can be tested and tests should be written for them.



geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommands.java
Line 810 (original), 728 (patched)
<https://reviews.apache.org/r/57243/#comment239628>

    At some point we need to delete CliUtil and CacheFactory.getAnyInstance and start passing around instances instead of using statics and singletons. This forces us into doing all of our testing with integration tests instead of unit tests.
    
    It's sad to see so many static calls in this method that are invoking getAnyInstance



geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommands.java
Line 833 (original), 733 (patched)
<https://reviews.apache.org/r/57243/#comment239629>

    At a minimum, I'd hoist this "Region region =" out of the for-loop and invoke it just once.



geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommands.java
Line 872 (original), 749 (patched)
<https://reviews.apache.org/r/57243/#comment239632>

    Fix typo "Recieved" and change to use log4j parms:
    ```java
    logger.info("Received zip file from member {}: {}", server.getId(), zipFile);
    ```



geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java
Lines 57 (patched)
<https://reviews.apache.org/r/57243/#comment239637>

    This class should be a Function that uses another class called LogExporter.
    
    LogExporter should be a class that follows solid OOP principles and has a new UnitTest that uses Mockito to test it in isolation.
    
    ExportLogsFunction then becomes an integration class that uses LogExporter within the context of a Function.



geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java
Lines 119 (patched)
<https://reviews.apache.org/r/57243/#comment239638>

    Why is this public? Is this called from other classes?



geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java
Lines 121 (patched)
<https://reviews.apache.org/r/57243/#comment239640>

    Let's get rid of these singleton calls. Make the methods non-static (and preferrably private). 
    
    Call GemFireCacheImpl.getInstance() once and store it in a member variable. Then have all of the other methods use that var instead of hitting GemFireCacheImpl.getInstance() repeatedly.
    
    We want to remove all statics and singletons so that we can start writing good unit tests.



geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java
Lines 142 (patched)
<https://reviews.apache.org/r/57243/#comment239639>

    Does this need to be public?



geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java
Lines 159 (patched)
<https://reviews.apache.org/r/57243/#comment239642>

    Can this be made private?



geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java
Lines 208 (patched)
<https://reviews.apache.org/r/57243/#comment239641>

    Can this be made private?


- Kirk Lund


On March 2, 2017, 4:32 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57243/
> -----------------------------------------------------------
> 
> (Updated March 2, 2017, 4:32 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> 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-2418: enable gfsh to download file from http connection
> GEODE-2267: add validation to the arguments and include export stats in the command
> GEODE-2267: use the config to determine where the logs and stats are
> GEODE-2267: Enhance server/locator startup rules to include workingDir
> 
> What's not done in this batch:
> 1. If you are exporting without http connection, exported zip will be left on the loator's working dir, currently we don't have a cleanup mechanism to delete them.
> 2. long running task may timeout the http connection. No test around this scenario.
> 3. No warning to the user if they are exporting a large quantity of logs.
> 4. we are exporting all the .log and .gfs files and only those files. Other file extensions are ignored.
> 5. merge-log option is ignored
> 
> 
> Diffs
> -----
> 
>   geode-assembly/build.gradle 1c95927f19888b15e771b3cbbc92f120787868c4 
>   geode-assembly/src/test/java/org/apache/geode/management/internal/AgentUtilJUnitTest.java 3a29e8a435f5d5362810f5e05ea92a1260e3fbfc 
>   geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java f0b6bb327f0424f2e4afa2e057d8e38bd0ef1b39 
>   geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java 160f6349d94646b12e3114da14e5bff569eed5a9 
>   geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java 10ce515df8c3630cbf01fc314be454d3e2adfe2c 
>   geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java c7d0e7341fcf13bf34a597405d2b42b608004215 
>   geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java b5472909eec8d5ca124e7bfd5c6cb71864d9bbee 
>   geode-core/src/main/java/org/apache/geode/distributed/DistributedSystem.java 20c948f8f16240aef5c8eae313164693125a99ca 
>   geode-core/src/main/java/org/apache/geode/internal/logging/GemFireLevel.java cffd162687573c9f7861bc2ada1076293c2e0a57 
>   geode-core/src/main/java/org/apache/geode/internal/logging/InternalLogWriter.java 661fce99f01a890b727daf7dc591316d1d8982fb 
>   geode-core/src/main/java/org/apache/geode/internal/logging/LogService.java 0ad7d848a05a0104ce01652b1fb628f2cff1b2d1 
>   geode-core/src/main/java/org/apache/geode/internal/statistics/StatArchiveHandler.java 03b0f898ad30b6b088108fa0187b0a5a750ed6dc 
>   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 8cd098d9719774c49efcf28d1d5ecdcd345687fc 
>   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/functions/LogFileFunction.java 41ffeb40b4dd09ed2bf7eb4ef2cd516001927025 
>   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/internal/statistics/SimpleStatSamplerIntegrationTest.java 167fa3d07802136ea68ec2f4f54e7c4716ea938a 
>   geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java 83a367eb88aec85984691e651e5de0f8b479c7cb 
>   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/HeadlessGfsh.java 72be0c7fa6839ff7f818cffa4ee5b436bebd2ce8 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CliCommandTestBase.java 21426d67904758aad4b1cf8b8c8966dd0fd45365 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java 05f2c0cb2115ee574666ed3da943c26e8c38c9f2 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptorJUnitTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsOnServerManagerDUnit.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportStatsDUnitTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart1DUnitTest.java cb9a8c6f74ba2a7b9799ad049a4ebe2861b9087b 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart2DUnitTest.java 2b2e524c5369950073099c3509c8f30e768607f6 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart3DUnitTest.java efef2c4abf70deeaef81285f13997542490234e3 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart4DUnitTest.java 9d64bd9ed3fec910c2b5dd4e8be50d3368c63893 
>   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/ExportLogsCacheWriterUnitTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterIntegrationTest.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/MergeLogsDUnitTest.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/CacheServerMBeanShiroJUnitTest.java 3183aafd9907ecb15714c2038f2b8c3af31e6e06 
>   geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java ea72cfab26f828af9a09f08755a9a7f9ed121654 
>   geode-core/src/test/java/org/apache/geode/management/internal/security/DeployCommandsSecurityTest.java e89bd62b59b8218c30c8f724b4febf2b3ae7721b 
>   geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java 0980c18de1b504f545355a0f7d650f46c92cc670 
>   geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java 2d32905d45496fe69e5dc189cd94b82ea163c3b5 
>   geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java 2ae61402d330ab6426c0bc0e0193771ce5dc9c70 
>   geode-core/src/test/java/org/apache/geode/management/internal/security/MultiUserDUnitTest.java 37e7f453dab01dbcfcc26ebacaf3c27cc680a5c8 
>   geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java 52afab40990ab3a18fed04fcb001ec9dbe7d045e 
>   geode-core/src/test/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java e87a285d088c923326aa82ee406f543ffc5e0593 
>   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/Locator.java fe26e8364798689c1ece487554d99085f3f26f23 
>   geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerConfigurationRule.java 0a6396263aa18629b178d82932be014a40c134a9 
>   geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java 68447201875ef31a73cfb888d346292e3f084ae8 
>   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/Server.java 4aa2c69389dce4650cf476f4e8decebddfc36736 
>   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/build.gradle ffab391d97792ce82fa21fecbc00afb45fe820d6 
>   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/ExportLogsOverHttpDUnitTest.java PRE-CREATION 
>   geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/QueryNamesOverHttpDUnitTest.java 1a19e5e5ca90c248655eb40e68aca3c7ed858770 
>   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/57243/diff/1/
> 
> 
> Testing
> -------
> 
> precheckin running (hopefully last one)
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 57243: GEODE-2416: Collect together artifacts from individual servers into a single zip file

Posted by Kevin Duling <kd...@pivotal.io>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57243/#review167900
-----------------------------------------------------------


Ship it!




Ship It!

- Kevin Duling


On March 3, 2017, 2:17 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57243/
> -----------------------------------------------------------
> 
> (Updated March 3, 2017, 2:17 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> 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-2418: enable gfsh to download file from http connection
> GEODE-2267: add validation to the arguments and include export stats in the command
> GEODE-2267: use the config to determine where the logs and stats are
> GEODE-2267: Enhance server/locator startup rules to include workingDir
> 
> What's not done in this batch:
> 1. If you are exporting without http connection, exported zip will be left on the loator's working dir, currently we don't have a cleanup mechanism to delete them.
> 2. long running task may timeout the http connection. No test around this scenario.
> 3. No warning to the user if they are exporting a large quantity of logs.
> 4. we are exporting all the .log and .gfs files and only those files. Other file extensions are ignored.
> 5. merge-log option is ignored
> 
> 
> Diffs
> -----
> 
>   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/CommandRequest.java b0242c9e8ee656001cf76155f4e727ece07666a2 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigCommands.java 12a0a64202ed56fb48e970066b9e8838b9d6bef3 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommands.java bd6772803b2f109b9ca0fc98e059c8a86464d85c 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportImportClusterConfigurationCommands.java e94af4f3f34e6fc43c86b23a942e30ac28d58505 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogCommand.java PRE-CREATION 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/FunctionCommands.java 91810e88ee19f938fd0c13116a4736e671d4df6a 
>   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/remote/RemoteExecutionStrategy.java 4b827a9e9022d06553ee2441ed50acd72252fb77 
>   geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java PRE-CREATION 
>   geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/MiscellaneousCommandsController.java ac912c82c873c443dd268f07e8874b5bd18fdd0b 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/AbstractCliAroundInterceptorJUnitTest.java c938f0755142209150a5a91a3764c54b009bb3a3 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptorJUnitTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportStatsDUnitTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunctionIntegrationTest.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/57243/diff/3/
> 
> 
> Testing
> -------
> 
> precheckin Successful with the latest changes
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 57243: GEODE-2416: Collect together artifacts from individual servers into a single zip file

Posted by Ken Howe <kh...@pivotal.io>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57243/#review167899
-----------------------------------------------------------


Ship it!




LGTM. I reviewed the diffs from the original to the latest (version 3) without examining the intermediate changes.

As these changes appear that they won't break nightly builds (your precheckin passed), it would be great to get this merged so that further refinements to the export logs command behavior can be dealt with individually.

- Ken Howe


On March 3, 2017, 10:17 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57243/
> -----------------------------------------------------------
> 
> (Updated March 3, 2017, 10:17 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> 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-2418: enable gfsh to download file from http connection
> GEODE-2267: add validation to the arguments and include export stats in the command
> GEODE-2267: use the config to determine where the logs and stats are
> GEODE-2267: Enhance server/locator startup rules to include workingDir
> 
> What's not done in this batch:
> 1. If you are exporting without http connection, exported zip will be left on the loator's working dir, currently we don't have a cleanup mechanism to delete them.
> 2. long running task may timeout the http connection. No test around this scenario.
> 3. No warning to the user if they are exporting a large quantity of logs.
> 4. we are exporting all the .log and .gfs files and only those files. Other file extensions are ignored.
> 5. merge-log option is ignored
> 
> 
> Diffs
> -----
> 
>   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/CommandRequest.java b0242c9e8ee656001cf76155f4e727ece07666a2 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigCommands.java 12a0a64202ed56fb48e970066b9e8838b9d6bef3 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommands.java bd6772803b2f109b9ca0fc98e059c8a86464d85c 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportImportClusterConfigurationCommands.java e94af4f3f34e6fc43c86b23a942e30ac28d58505 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogCommand.java PRE-CREATION 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/FunctionCommands.java 91810e88ee19f938fd0c13116a4736e671d4df6a 
>   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/remote/RemoteExecutionStrategy.java 4b827a9e9022d06553ee2441ed50acd72252fb77 
>   geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java PRE-CREATION 
>   geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/MiscellaneousCommandsController.java ac912c82c873c443dd268f07e8874b5bd18fdd0b 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/AbstractCliAroundInterceptorJUnitTest.java c938f0755142209150a5a91a3764c54b009bb3a3 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptorJUnitTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportStatsDUnitTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunctionIntegrationTest.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/57243/diff/3/
> 
> 
> Testing
> -------
> 
> precheckin Successful with the latest changes
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 57243: GEODE-2416: Collect together artifacts from individual servers into a single zip file

Posted by Jared Stewart <js...@pivotal.io>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57243/#review167906
-----------------------------------------------------------


Ship it!




Ship It!

- Jared Stewart


On March 3, 2017, 10:17 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57243/
> -----------------------------------------------------------
> 
> (Updated March 3, 2017, 10:17 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> 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-2418: enable gfsh to download file from http connection
> GEODE-2267: add validation to the arguments and include export stats in the command
> GEODE-2267: use the config to determine where the logs and stats are
> GEODE-2267: Enhance server/locator startup rules to include workingDir
> 
> What's not done in this batch:
> 1. If you are exporting without http connection, exported zip will be left on the loator's working dir, currently we don't have a cleanup mechanism to delete them.
> 2. long running task may timeout the http connection. No test around this scenario.
> 3. No warning to the user if they are exporting a large quantity of logs.
> 4. we are exporting all the .log and .gfs files and only those files. Other file extensions are ignored.
> 5. merge-log option is ignored
> 
> 
> Diffs
> -----
> 
>   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/CommandRequest.java b0242c9e8ee656001cf76155f4e727ece07666a2 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigCommands.java 12a0a64202ed56fb48e970066b9e8838b9d6bef3 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommands.java bd6772803b2f109b9ca0fc98e059c8a86464d85c 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportImportClusterConfigurationCommands.java e94af4f3f34e6fc43c86b23a942e30ac28d58505 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogCommand.java PRE-CREATION 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/FunctionCommands.java 91810e88ee19f938fd0c13116a4736e671d4df6a 
>   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/remote/RemoteExecutionStrategy.java 4b827a9e9022d06553ee2441ed50acd72252fb77 
>   geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java PRE-CREATION 
>   geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/MiscellaneousCommandsController.java ac912c82c873c443dd268f07e8874b5bd18fdd0b 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/AbstractCliAroundInterceptorJUnitTest.java c938f0755142209150a5a91a3764c54b009bb3a3 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptorJUnitTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportStatsDUnitTest.java PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunctionIntegrationTest.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/57243/diff/3/
> 
> 
> Testing
> -------
> 
> precheckin Successful with the latest changes
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 57243: GEODE-2416: Collect together artifacts from individual servers into a single zip file

Posted by Jinmei Liao <ji...@pivotal.io>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57243/
-----------------------------------------------------------

(Updated March 3, 2017, 10:17 p.m.)


Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.


Repository: geode


Description
-------

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-2418: enable gfsh to download file from http connection
GEODE-2267: add validation to the arguments and include export stats in the command
GEODE-2267: use the config to determine where the logs and stats are
GEODE-2267: Enhance server/locator startup rules to include workingDir

What's not done in this batch:
1. If you are exporting without http connection, exported zip will be left on the loator's working dir, currently we don't have a cleanup mechanism to delete them.
2. long running task may timeout the http connection. No test around this scenario.
3. No warning to the user if they are exporting a large quantity of logs.
4. we are exporting all the .log and .gfs files and only those files. Other file extensions are ignored.
5. merge-log option is ignored


Diffs
-----

  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/CommandRequest.java b0242c9e8ee656001cf76155f4e727ece07666a2 
  geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigCommands.java 12a0a64202ed56fb48e970066b9e8838b9d6bef3 
  geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommands.java bd6772803b2f109b9ca0fc98e059c8a86464d85c 
  geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportImportClusterConfigurationCommands.java e94af4f3f34e6fc43c86b23a942e30ac28d58505 
  geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogCommand.java PRE-CREATION 
  geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/FunctionCommands.java 91810e88ee19f938fd0c13116a4736e671d4df6a 
  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/remote/RemoteExecutionStrategy.java 4b827a9e9022d06553ee2441ed50acd72252fb77 
  geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java PRE-CREATION 
  geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/MiscellaneousCommandsController.java ac912c82c873c443dd268f07e8874b5bd18fdd0b 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/AbstractCliAroundInterceptorJUnitTest.java c938f0755142209150a5a91a3764c54b009bb3a3 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptorJUnitTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportStatsDUnitTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunctionIntegrationTest.java PRE-CREATION 


Diff: https://reviews.apache.org/r/57243/diff/3/


Testing (updated)
-------

precheckin Successful with the latest changes


Thanks,

Jinmei Liao

Re: Review Request 57243: GEODE-2416: Collect together artifacts from individual servers into a single zip file

Posted by Jinmei Liao <ji...@pivotal.io>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57243/
-----------------------------------------------------------

(Updated March 3, 2017, 4:17 p.m.)


Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.


Repository: geode


Description
-------

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-2418: enable gfsh to download file from http connection
GEODE-2267: add validation to the arguments and include export stats in the command
GEODE-2267: use the config to determine where the logs and stats are
GEODE-2267: Enhance server/locator startup rules to include workingDir

What's not done in this batch:
1. If you are exporting without http connection, exported zip will be left on the loator's working dir, currently we don't have a cleanup mechanism to delete them.
2. long running task may timeout the http connection. No test around this scenario.
3. No warning to the user if they are exporting a large quantity of logs.
4. we are exporting all the .log and .gfs files and only those files. Other file extensions are ignored.
5. merge-log option is ignored


Diffs (updated)
-----

  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/CommandRequest.java b0242c9e8ee656001cf76155f4e727ece07666a2 
  geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigCommands.java 12a0a64202ed56fb48e970066b9e8838b9d6bef3 
  geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommands.java bd6772803b2f109b9ca0fc98e059c8a86464d85c 
  geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportImportClusterConfigurationCommands.java e94af4f3f34e6fc43c86b23a942e30ac28d58505 
  geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogCommand.java PRE-CREATION 
  geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/FunctionCommands.java 91810e88ee19f938fd0c13116a4736e671d4df6a 
  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/remote/RemoteExecutionStrategy.java 4b827a9e9022d06553ee2441ed50acd72252fb77 
  geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java PRE-CREATION 
  geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/MiscellaneousCommandsController.java ac912c82c873c443dd268f07e8874b5bd18fdd0b 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/AbstractCliAroundInterceptorJUnitTest.java c938f0755142209150a5a91a3764c54b009bb3a3 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptorJUnitTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportStatsDUnitTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunctionIntegrationTest.java PRE-CREATION 


Diff: https://reviews.apache.org/r/57243/diff/3/

Changes: https://reviews.apache.org/r/57243/diff/2-3/


Testing
-------

precheckin running (hopefully last one)


Thanks,

Jinmei Liao

Re: Review Request 57243: GEODE-2416: Collect together artifacts from individual servers into a single zip file

Posted by Jinmei Liao <ji...@pivotal.io>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57243/
-----------------------------------------------------------

(Updated March 2, 2017, 7:26 p.m.)


Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.


Changes
-------

precheckin successful. I rebased the changeset to the current develop, so that you won't be getting the changes of the rules refactoring. hopefully the changeset would be smaller.


Repository: geode


Description
-------

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-2418: enable gfsh to download file from http connection
GEODE-2267: add validation to the arguments and include export stats in the command
GEODE-2267: use the config to determine where the logs and stats are
GEODE-2267: Enhance server/locator startup rules to include workingDir

What's not done in this batch:
1. If you are exporting without http connection, exported zip will be left on the loator's working dir, currently we don't have a cleanup mechanism to delete them.
2. long running task may timeout the http connection. No test around this scenario.
3. No warning to the user if they are exporting a large quantity of logs.
4. we are exporting all the .log and .gfs files and only those files. Other file extensions are ignored.
5. merge-log option is ignored


Diffs (updated)
-----

  geode-core/src/main/java/org/apache/geode/internal/logging/GemFireLevel.java cffd162687573c9f7861bc2ada1076293c2e0a57 
  geode-core/src/main/java/org/apache/geode/internal/logging/InternalLogWriter.java 661fce99f01a890b727daf7dc591316d1d8982fb 
  geode-core/src/main/java/org/apache/geode/internal/logging/LogService.java 0ad7d848a05a0104ce01652b1fb628f2cff1b2d1 
  geode-core/src/main/java/org/apache/geode/internal/statistics/StatArchiveHandler.java 03b0f898ad30b6b088108fa0187b0a5a750ed6dc 
  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 8cd098d9719774c49efcf28d1d5ecdcd345687fc 
  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/functions/LogFileFunction.java 41ffeb40b4dd09ed2bf7eb4ef2cd516001927025 
  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/internal/statistics/SimpleStatSamplerIntegrationTest.java 167fa3d07802136ea68ec2f4f54e7c4716ea938a 
  geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java 83a367eb88aec85984691e651e5de0f8b479c7cb 
  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/HeadlessGfsh.java 76e986df2c69bfde9d7311138f8727610276876f 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CliCommandTestBase.java 21426d67904758aad4b1cf8b8c8966dd0fd45365 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptorJUnitTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsOnServerManagerDUnit.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportStatsDUnitTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart1DUnitTest.java cb9a8c6f74ba2a7b9799ad049a4ebe2861b9087b 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart2DUnitTest.java 2b2e524c5369950073099c3509c8f30e768607f6 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart3DUnitTest.java efef2c4abf70deeaef81285f13997542490234e3 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart4DUnitTest.java 9d64bd9ed3fec910c2b5dd4e8be50d3368c63893 
  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/ExportLogsCacheWriterUnitTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterIntegrationTest.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/MergeLogsDUnitTest.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 d1750c3a5efd636c0f9f47fabe670265bf46f072 
  geode-core/src/test/java/org/apache/geode/management/internal/security/MultiUserDUnitTest.java 37e7f453dab01dbcfcc26ebacaf3c27cc680a5c8 
  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/LocatorServerConfigurationRule.java 0a6396263aa18629b178d82932be014a40c134a9 
  geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java 39c13d000db80bef37563729bc17ae4bcb566153 
  geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java 84c660cfe0ab9b7c6ba0bfc08b034ccaf17697f0 
  geode-core/src/test/java/org/apache/geode/test/dunit/rules/Member.java 5f46da21f380a67a8e7f4855da1dfbb3118057ba 
  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/build.gradle ffab391d97792ce82fa21fecbc00afb45fe820d6 
  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/ExportLogsOverHttpDUnitTest.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/57243/diff/2/

Changes: https://reviews.apache.org/r/57243/diff/1-2/


Testing
-------

precheckin running (hopefully last one)


Thanks,

Jinmei Liao

Re: Review Request 57243: GEODE-2416: Collect together artifacts from individual servers into a single zip file

Posted by Jinmei Liao <ji...@pivotal.io>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57243/
-----------------------------------------------------------

(Updated March 2, 2017, 4:32 p.m.)


Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.


Repository: geode


Description (updated)
-------

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-2418: enable gfsh to download file from http connection
GEODE-2267: add validation to the arguments and include export stats in the command
GEODE-2267: use the config to determine where the logs and stats are
GEODE-2267: Enhance server/locator startup rules to include workingDir

What's not done in this batch:
1. If you are exporting without http connection, exported zip will be left on the loator's working dir, currently we don't have a cleanup mechanism to delete them.
2. long running task may timeout the http connection. No test around this scenario.
3. No warning to the user if they are exporting a large quantity of logs.
4. we are exporting all the .log and .gfs files and only those files. Other file extensions are ignored.
5. merge-log option is ignored


Diffs
-----

  geode-assembly/build.gradle 1c95927f19888b15e771b3cbbc92f120787868c4 
  geode-assembly/src/test/java/org/apache/geode/management/internal/AgentUtilJUnitTest.java 3a29e8a435f5d5362810f5e05ea92a1260e3fbfc 
  geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java f0b6bb327f0424f2e4afa2e057d8e38bd0ef1b39 
  geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java 160f6349d94646b12e3114da14e5bff569eed5a9 
  geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java 10ce515df8c3630cbf01fc314be454d3e2adfe2c 
  geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java c7d0e7341fcf13bf34a597405d2b42b608004215 
  geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java b5472909eec8d5ca124e7bfd5c6cb71864d9bbee 
  geode-core/src/main/java/org/apache/geode/distributed/DistributedSystem.java 20c948f8f16240aef5c8eae313164693125a99ca 
  geode-core/src/main/java/org/apache/geode/internal/logging/GemFireLevel.java cffd162687573c9f7861bc2ada1076293c2e0a57 
  geode-core/src/main/java/org/apache/geode/internal/logging/InternalLogWriter.java 661fce99f01a890b727daf7dc591316d1d8982fb 
  geode-core/src/main/java/org/apache/geode/internal/logging/LogService.java 0ad7d848a05a0104ce01652b1fb628f2cff1b2d1 
  geode-core/src/main/java/org/apache/geode/internal/statistics/StatArchiveHandler.java 03b0f898ad30b6b088108fa0187b0a5a750ed6dc 
  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 8cd098d9719774c49efcf28d1d5ecdcd345687fc 
  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/functions/LogFileFunction.java 41ffeb40b4dd09ed2bf7eb4ef2cd516001927025 
  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/internal/statistics/SimpleStatSamplerIntegrationTest.java 167fa3d07802136ea68ec2f4f54e7c4716ea938a 
  geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java 83a367eb88aec85984691e651e5de0f8b479c7cb 
  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/HeadlessGfsh.java 72be0c7fa6839ff7f818cffa4ee5b436bebd2ce8 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CliCommandTestBase.java 21426d67904758aad4b1cf8b8c8966dd0fd45365 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java 05f2c0cb2115ee574666ed3da943c26e8c38c9f2 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptorJUnitTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsOnServerManagerDUnit.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportStatsDUnitTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart1DUnitTest.java cb9a8c6f74ba2a7b9799ad049a4ebe2861b9087b 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart2DUnitTest.java 2b2e524c5369950073099c3509c8f30e768607f6 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart3DUnitTest.java efef2c4abf70deeaef81285f13997542490234e3 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommandsExportLogsPart4DUnitTest.java 9d64bd9ed3fec910c2b5dd4e8be50d3368c63893 
  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/ExportLogsCacheWriterUnitTest.java PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterIntegrationTest.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/MergeLogsDUnitTest.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/CacheServerMBeanShiroJUnitTest.java 3183aafd9907ecb15714c2038f2b8c3af31e6e06 
  geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java ea72cfab26f828af9a09f08755a9a7f9ed121654 
  geode-core/src/test/java/org/apache/geode/management/internal/security/DeployCommandsSecurityTest.java e89bd62b59b8218c30c8f724b4febf2b3ae7721b 
  geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java 0980c18de1b504f545355a0f7d650f46c92cc670 
  geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java 2d32905d45496fe69e5dc189cd94b82ea163c3b5 
  geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java 2ae61402d330ab6426c0bc0e0193771ce5dc9c70 
  geode-core/src/test/java/org/apache/geode/management/internal/security/MultiUserDUnitTest.java 37e7f453dab01dbcfcc26ebacaf3c27cc680a5c8 
  geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java 52afab40990ab3a18fed04fcb001ec9dbe7d045e 
  geode-core/src/test/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java e87a285d088c923326aa82ee406f543ffc5e0593 
  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/Locator.java fe26e8364798689c1ece487554d99085f3f26f23 
  geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerConfigurationRule.java 0a6396263aa18629b178d82932be014a40c134a9 
  geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java 68447201875ef31a73cfb888d346292e3f084ae8 
  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/Server.java 4aa2c69389dce4650cf476f4e8decebddfc36736 
  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/build.gradle ffab391d97792ce82fa21fecbc00afb45fe820d6 
  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/ExportLogsOverHttpDUnitTest.java PRE-CREATION 
  geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/QueryNamesOverHttpDUnitTest.java 1a19e5e5ca90c248655eb40e68aca3c7ed858770 
  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/57243/diff/1/


Testing
-------

precheckin running (hopefully last one)


Thanks,

Jinmei Liao