> On July 11, 2016, 2:19 p.m., Mike Percy wrote: > > flume-ng-configuration/src/main/java/org/apache/flume/Context.java, line 85 > > <https://reviews.apache.org/r/49458/diff/6/?file=1440353#file1440353line85> > > > > This is an API and ABI breaking change. > > > > We would need to release Flume 2.0 to do this. > > Lior Zeno wrote: > Guava is shaded, it's problematic to expose a shaded jar in the API. > > Lior Zeno wrote: > I must add that I think it's a bad practice to expose a foreign namespace > in our API. Either org.apache.flume.X or native java namespaces are > acceptable. Exposing a foreign namespace results in exactly this problem. We > are now going to break the API because of this.
It's really unfortunate that we have exposed Guava in the API. I had lunch with Hari today and he was surprised as well. I think this was an oversight on our part at the time and something we didn't really intend. We also didn't predict that Guava would be so backwards incompatible. Guava is quite dangerous, and in hindsight that is very clear now. Sadly, now I think we are stuck with it. We cannot break API compatibility within the 1.x line. We should follow the standard at http://semver.org in this regard. Overall I am probably nearly as frustrated with this problem as you are. I don't think there is a clean way out of this. Some hacky options come to mind in terms of how to move forward. None of them are very good. I'll post them on the JIRA and we can discuss our options there. - Mike ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49458/#review141781 ----------------------------------------------------------- On July 9, 2016, 3:13 a.m., Lior Zeno wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49458/ > ----------------------------------------------------------- > > (Updated July 9, 2016, 3:13 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-configuration/src/main/java/org/apache/flume/Context.java f00b571 > > flume-ng-core/src/main/java/org/apache/flume/source/MultiportSyslogTCPSource.java > b9f2438 > flume-ng-dist/src/main/assembly/bin.xml a61180d > flume-ng-doc/sphinx/FlumeUserGuide.rst f9ca1b2 > > flume-ng-embedded-agent/src/test/java/org/apache/flume/agent/embedded/TestEmbeddedAgentEmbeddedSource.java > c122a12 > > flume-ng-node/src/main/java/org/apache/flume/node/MaterializedConfiguration.java > a80bfdf > 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 > 9fbd747 > > 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 > b62254e > > flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchLogStashEventSerializer.java > 65b4dab > > flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java > 69acc06 > pom.xml b50693e > > 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 > >
