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




geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
Lines 145 (patched)
<https://reviews.apache.org/r/59287/#comment249282>

    is this method called anywhere else with a different first parameter? 
    
    If only called once, seems redundant that the method throws an exception 
and the caller directly catches that exception only to get the message. We can 
either directly build the message here or have the method return the message.



geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
Lines 203 (patched)
<https://reviews.apache.org/r/59287/#comment249280>

    We've already passed the size check before, and already generated the zip 
on the locator, I don't think we need this extra check here.


- Jinmei Liao


On May 23, 2017, 3:45 p.m., Ken Howe wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59287/
> -----------------------------------------------------------
> 
> (Updated May 23, 2017, 3:45 p.m.)
> 
> 
> Review request for geode, Jinmei Liao, Jared Stewart, Kirk Lund, and Patrick 
> Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> Adds 'export logs' option, --file-limit-size, to allow user to set
> maximun size of the epxorted logs zip file.
> 
> When size checking is enabled (file-limit-size > 0) then the check
> will also prevent filling up the disk on each member while consolidating
> and filtering the logs.
> 
> 
> Diffs
> -----
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/InternalDistributedMember.java
>  7170f209ffa169fb6efdc851d35b61a2031888b7 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
>  20ec1f5702aea341ace5aa3b103c34cbdce1ae87 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java
>  663a08e15624ed3dbc032460133fe3c62fc5ac26 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfo.java
>  c175e1ae3def869890692461bd129891350b383c 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java
>  8d20dc05c14bf558462893c4dd4cbbc474df4077 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
>  68d055cbd61ca35ef7409ff3370214a005da3d9b 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java
>  a0be7fbd83918fccb5254b4a48ba7bf14a0fb344 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java
>  0a799f6c85dada2791da57585234fa2e47ef0b3d 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java
>  a02c07f2c28156e097306f4b57174cddeda78845 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java
>  96ac76588662b1de5d5bf41c24ab115d90fc0a85 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java
>  ec2bcfe8ea876172c6946c43c005659d23d055e0 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java
>  90a92f33247ecec8ee300ecb80a5d8ab27193c94 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfoTest.java
>  0bfbefa90af7813a8cf20529d36c9cbe5111f9d9 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java
>  d8f2f2db937fc51ab5f917659e766f338b9ae847 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsTestSuite.java
>  e70a750f48a8f7cc3b10b89be9b5934944addb0d 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterTest.java
>  a387af3b70f61256b5d9303de29e9402bbdd71e6 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogSizerTest.java
>  c7b3ab934ce1cf97c0a70bc23b50f0f5ca08bb40 
> 
> 
> Diff: https://reviews.apache.org/r/59287/diff/5/
> 
> 
> Testing
> -------
> 
> Precheckin is in progress - all green so far with only DistributedTest still 
> running
> 
> 5/22 - Precheckin being re-run. Currently Green expcet for the known 
> LocatorLauncher test failures. DistributedTests still running
> 
> 5/23 - Re-running precheckin after syncing with current state of develop
> 
> 
> Thanks,
> 
> Ken Howe
> 
>

Reply via email to