----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58010/#review170470 -----------------------------------------------------------
I would add a new constant for DEFAULT_EXPORT_LOG_LEVEL, and move it to ExportLogsCommand. This makes it completely clear that the level we're filtering on for exporting the logs is completely different than any level for what is getting logged. I also think that LogService.DEFAULT_LOG_LEVEL should be removed; it's internal and not used anywhere (after the change noted above.) - Ken Howe On March 28, 2017, 10:34 p.m., Patrick Rhomberg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58010/ > ----------------------------------------------------------- > > (Updated March 28, 2017, 10:34 p.m.) > > > Review request for geode, Jinmei Liao, Jared Stewart, Kevin Duling, Ken Howe, > Kirk Lund, and Swapnil Bawaskar. > > > Repository: geode > > > Description > ------- > > Added DUnit test that fails under previous behavior. > > > Diffs > ----- > > geode-core/src/main/java/org/apache/geode/internal/logging/LogService.java > 1f8a564a8111b036f5e5cce6393931c714985c9c > > geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java > cbdf1c4bc28554a8fbec3740c566ee07c69b4ac9 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java > 95edd426da8b8f39bb1486661d8c307d43f170d6 > > > Diff: https://reviews.apache.org/r/58010/diff/1/ > > > Testing > ------- > > Added DUnit passes under new behavior. > > === > > While `LogService.DEFAULT_LOG_LEVEL` is only used by the export logs command, > I don't know if that is the best place to make this update. Should that stay > `INFO` and a new variable `DEFAULT_EXPORT_LOG_LEVEL` be established? > > > Thanks, > > Patrick Rhomberg > >
