flume-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Lior Zeno <liorz...@gmail.com>
Subject Re: Review Request 49458: FLUME-2921 Support Elasticsearch 2.0+
Date Tue, 05 Jul 2016 16:51:59 GMT


> On July 5, 2016, 8:27 a.m., Attila Simon wrote:
> > Ship It!

Thanks. I must say that I'm not feeling completely comfortable with this patch. I am concerned
about the shaded elasticsearch project. Basically, this is just a workaround the guava collision
issue. There is a systematic way to deal with this, which is to remove guava as a dependency
since it's likely to collide with other dependencies. See FLUME-2944.
Do you think we should tackle this on 1.7.0?


- Lior


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


On July 2, 2016, 8:04 a.m., Lior Zeno wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49458/
> -----------------------------------------------------------
> 
> (Updated July 2, 2016, 8:04 a.m.)
> 
> 
> Review request for Flume.
> 
> 
> Bugs: FLUME-2921
>     https://issues.apache.org/jira/browse/FLUME-2921
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> This patch adds the support for Elasticsearch version 2.0+. The version I used is 2.3.3,
which is the latest stable release.
> This patch does not fix any known issues with this sink, its only purpose is to support
current versions of elasticsearch.
> 
> Elasticsearch 2.3.3 depends on guava 18.0, which collided with our version. I had to
create a new module, flume-ng-elasticsearch-shaded, and shade guava. This worked this time,
but due to guava's popularity I think we should remove this dependency in the future. This
should be easier, now that Flume uses Java 1.7.
> 
> 
> Diffs
> -----
> 
>   flume-ng-dist/pom.xml 7fdf36a 
>   flume-ng-doc/sphinx/FlumeUserGuide.rst c4d7c6c 
>   flume-ng-sinks/flume-ng-elasticsearch-shaded/pom.xml e69de29 
>   flume-ng-sinks/flume-ng-elasticsearch-sink/pom.xml c372c0b 
>   flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ContentBuilderUtil.java
83c3ffd 
>   flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchLogStashEventSerializer.java
3638368 
>   flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/client/ElasticSearchTransportClient.java
2cf365e 
>   flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchSinkTest.java
f9272fa 
>   flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchDynamicSerializer.java
d4e4654 
>   flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchIndexRequestBuilderFactory.java
8022111 
>   flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchLogStashEventSerializer.java
ab9587d 
>   flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java
a58f344 
>   flume-ng-sinks/pom.xml 00791e4 
>   pom.xml 85c0dc8 
> 
> Diff: https://reviews.apache.org/r/49458/diff/
> 
> 
> Testing
> -------
> 
> I made sure that all unit tests (due to guava upgrade) pass successfully. The known flaky
tests may not pass, though.
> In addition, I tested the sink against a local elasticsearch instance.
> 
> 
> Thanks,
> 
> Lior Zeno
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message