----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49458/#review141333 -----------------------------------------------------------
flume-ng-sinks/flume-ng-elasticsearch-shaded/pom.xml (line 25) <https://reviews.apache.org/r/49458/#comment206843> I am confused about the purpose of this module. What is being shaded? In my experience, shading only works if the dependencies are hidden inside a module, i.e. Guava is used internally but not exposed via APIs. Is that the case here? Keep in mind that if both ElasticSearchSink and HDFSEventSink are used in the same Flume agent, there must be a single version of the Guava JAR loaded, and those two projects must both work with it. pom.xml (line 784) <https://reviews.apache.org/r/49458/#comment206841> If you are going to include a new plugin, its version should go into the pluginManagement section. However, see my other comments - Mike Percy 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 > >
