----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57617/#review180678 -----------------------------------------------------------
Ship it! Thank you for this path. I tested the patch with your solution which seems to be good. - Miklos Csanady On March 16, 2017, 8 a.m., Marcell Hegedus wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57617/ > ----------------------------------------------------------- > > (Updated March 16, 2017, 8 a.m.) > > > Review request for Flume. > > > Bugs: FLUME-2620 > https://issues.apache.org/jira/browse/FLUME-2620 > > > Repository: flume-git > > > Description > ------- > > Flume user guide does not specify whether a value in event header could be > null or not. Given an external system generating events which header vaules > can be null and a user configures Flume with Memory Channel then he will have > no trouble. Later on when the user changes Memory Channel to File Channel > then Flume will fail with NPE. It is because FC is serializing events with > protocol buffer and header values are defined as required in the proto file. > In this patch I have changed the value field to optional. However protocol > buffer does not have a notation for null and setting a field to null raises > NPE again. Added a null check before serialization to prevent this. > There is on caveat: When an optional field is not set, at deserialization it > will be set to a default value: in this case it will be empty string. > > > Diffs > ----- > > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Put.java > 0a70a24 > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/proto/ProtosFactory.java > 50492cc > flume-ng-channels/flume-file-channel/src/main/proto/filechannel.proto > 25520e8 > > flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFileChannel.java > 8efe991 > flume-ng-core/src/test/java/org/apache/flume/channel/TestMemoryChannel.java > 344bb58 > > > Diff: https://reviews.apache.org/r/57617/diff/1/ > > > Testing > ------- > > Added unit test for both Memory Channel and File Channel to check if they > accept null values in header. > > > Thanks, > > Marcell Hegedus > >
