----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3962/#review6159 -----------------------------------------------------------
Changes look good Brock. Some minor feedback: * Please remove the extra whitespaces/tabs that are highlighted as red on the review flume-ng-core/src/main/java/org/apache/flume/source/ExecSource.java <https://reviews.apache.org/r/3962/#comment13244> Please create a ExecSourceConfigurationConstants class and declare these strings as public static final String CONFIG_RESTART_THROTTLE etc. The patch will also need to be rebased on the trunk as the sources have changed a bit since the last time. - Arvind On 2012-02-20 14:21:02, Brock Noland wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3962/ > ----------------------------------------------------------- > > (Updated 2012-02-20 14:21:02) > > > Review request for Flume. > > > Summary > ------- > > 1) Allows users to configure a restart flag and sleep interval for the > ExecSource should the command exit. > 2) Currently no one is reading standard error for the sub process. It is not > ignored like the documentation suggests. If the command generates enough > stderr to fill it's buffer, it will be blocked until someone reads that > buffer. A thread is added which by default reads and throws away stderr. If a > flag is turned on it will logstderr. This kind of flag can be very useful > when debugging why a command is not outputting the expected data. > > > This addresses bug FLUME-979. > https://issues.apache.org/jira/browse/FLUME-979 > > > Diffs > ----- > > flume-ng-core/src/test/java/org/apache/flume/source/TestExecSource.java > 3014368 > flume-ng-core/src/main/java/org/apache/flume/source/ExecSource.java a96016c > > Diff: https://reviews.apache.org/r/3962/diff > > > Testing > ------- > > A test for the restart flag and restartThrottle flag were added. All unit > tests pass. > > > Thanks, > > Brock > >
