> On May 8, 2016, 9:16 p.m., Jarek Cecho wrote:
> > flume-ng-core/src/main/java/org/apache/flume/source/http/HTTPSourceHandler.java,
> >  line 44
> > <https://reviews.apache.org/r/47098/diff/1/?file=1375930#file1375930line44>
> >
> >     This will be a backward in-compatible change. Can we perhaps convert 
> > this interface to abstract class, move handling of the nullReplacement 
> > here, so that we will break the contract only once?
> >     
> >     E.g. that we won't have to break it somewhere in the future when we 
> > will need to add yet another argument.

Unfortunately, changing this to an abstract class does not solve the issue here 
- since that is a bytecode change and breaks compat (code too - implements vs 
extends). One way of fixing this is to add an abstract class that inherits this 
one, and have an instance of check in the Source itself, and call this method 
only if the handler is an instance of the abstract class.


- Hari


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


On May 9, 2016, 4:57 a.m., neerja khattar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47098/
> -----------------------------------------------------------
> 
> (Updated May 9, 2016, 4:57 a.m.)
> 
> 
> Review request for Flume.
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> The issue is when the header value is null it throws null pointer exception 
> and flume stops processing further events.
> For example:
> [{
>   "headers" : {
>              "timestamp" : "434324343",
>              "host" : null
>              },
>   "body" : "random_body"
>   }]
>   
>   The solution to fix this is:
>   
>   1. If the header has a null value in the json, flume will replace it with a 
> replacement string.
>   2. The default value for a replacement string is an empty string.
>   3. To overwrite default string, set "handler.nullReplacementHeader" 
> property in flume config.
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/source/http/HTTPSource.java 
> b520b03 
>   
> flume-ng-core/src/main/java/org/apache/flume/source/http/HTTPSourceConfigurationConstants.java
>  86caf7d 
>   
> flume-ng-core/src/main/java/org/apache/flume/source/http/HTTPSourceHandler.java
>  726bf0c 
>   flume-ng-core/src/main/java/org/apache/flume/source/http/JSONHandler.java 
> 197f66a 
>   
> flume-ng-core/src/test/java/org/apache/flume/source/http/TestJSONHandler.java 
> 455781c 
>   
> flume-ng-sinks/flume-ng-morphline-solr-sink/src/main/java/org/apache/flume/sink/solr/morphline/BlobHandler.java
>  e84dec1 
> 
> Diff: https://reviews.apache.org/r/47098/diff/
> 
> 
> Testing
> -------
> 
> The following are the test cases:
> 
> 1. Header has null value in json and handler.nullReplacementHeader is not set 
> in flume config. The default value will be used to replace null.  
> 
> [{
>   "headers" : {
>              "timestamp" : "434324343",
>              "host" : null
>              },
>   "body" : "random_body"
>   }]
>   
>   Output in hdfs : {timestamp=434324343, host=} random_body  
>   
>   2. Header is not null in json and handler.nullReplacementHeader is not set 
> in flume config. The replacement implementation doesnt come in to 
> consideration.
>    
>   [{
>   "headers" : {
>              "timestamp" : "434324343",
>              "host" : 1
>              },
>   "body" : "random_body"
>   }]
>   Output in hdfs : {timestamp=434324343, host=1} random_body 
>   
>   3. Header has null value in json and handler.nullReplacementHeader=abc is 
> set in flume config. The null value in header will be replaced by abc.
> 
>   
>   [{
>   "headers" : {
>              "timestamp" : "434324343",
>              "host" : null
>              },
>   "body" : "random_body"
>   }]
>   
>  
>   Output in hdfs {timestamp=434324343, host=abc} random_body 
>   
>   4. Header has null value in json and handler.nullReplacementHeader=1 is set 
> in flume config. The null value in header will be replaced by 1 as a string .
>   
>   [{
>   "headers" : {
>              "timestamp" : "434324343",
>              "host" : null
>              },
>   "body" : "random_body"
>   }]
>   
>  
>   Output in hdfs: {timestamp=434324343, host=1} random_body
>   
>   5. Header is not null in json and handler.nullReplacementHeader is also set 
> in flume config. The replacement implementation doesnt come in to 
> consideration.
>    
>   [{
>   "headers" : {
>              "timestamp" : "434324343",
>              "host" : 1
>              },
>   "body" : "random_body"
>   }]
>   Output in hdfs : {timestamp=434324343, host=1} random_body
> 
> 
> File Attachments
> ----------------
> 
> flume-2620
>   
> https://reviews.apache.org/media/uploaded/files/2016/05/09/0eff1d56-caf3-4d36-bb45-6b9e7fd6a1ff__FLUME-2620-1.patch
> 
> 
> Thanks,
> 
> neerja khattar
> 
>

Reply via email to