> On May 16, 2017, 5:19 p.m., Jared Stewart wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
> > Line 230 (original), 300 (patched)
> > <https://reviews.apache.org/r/59287/diff/2/?file=1720360#file1720360line305>
> >
> >     It doesn't look like the return values of this method or the method on 
> > 319 are ever used in the product code.  Does it make sense to have these 
> > return a boolean rather than void?

Reverted this method to a void type and refactored tests.


> On May 16, 2017, 5:19 p.m., Jared Stewart wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
> > Line 241 (original), 319 (patched)
> > <https://reviews.apache.org/r/59287/diff/2/?file=1720360#file1720360line324>
> >
> >     The `diskSize` parameter in this method looks to be unused.

After refactoring to do the size check on on each member instead of back at the 
locator, the only item returned from SizeExportLogFunction is the estimated 
size for that member. Consequently, I've removed the ExportedLogsSizeInfo, and 
now just retiurn a single Long as the result (or error if the size check fails).


- Ken


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


On May 22, 2017, 6:14 p.m., Ken Howe wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59287/
> -----------------------------------------------------------
> 
> (Updated May 22, 2017, 6:14 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/3/
> 
> 
> 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
> 
> 
> Thanks,
> 
> Ken Howe
> 
>

Reply via email to