> On July 17, 2017, 4:20 p.m., Miklos Csanady wrote:
> > Thank you for this path.
> > 
> > I tested the patch with your solution which seems to be good.

Thank you for the patch Marcell and for the review Miklos, I'm going to commit 
this if nobody has any concerns.


- Denes


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


On March 16, 2017, 9 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, 9 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
> 
>

Reply via email to