----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24213/#review50203 -----------------------------------------------------------
I have a small question about the division of responsibilities with the factory and some minor doc updates. Otherwise this is a useful and clean change. flume-ng-doc/sphinx/FlumeUserGuide.rst <https://reviews.apache.org/r/24213/#comment87813> I think we should state what interface the indexNameBuilder has to implement. flume-ng-doc/sphinx/FlumeUserGuide.rst <https://reviews.apache.org/r/24213/#comment87814> I think we should state what interface the ElasticSearchClient has to implement. flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java <https://reviews.apache.org/r/24213/#comment87818> What is the purpose of the factory if we then take its output and do further configuration on it? Can we not have the factory do the complete initialisation or is there a reason not to? - Edward Sargisson On Aug. 5, 2014, 3:13 a.m., Bastian Germann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24213/ > ----------------------------------------------------------- > > (Updated Aug. 5, 2014, 3:13 a.m.) > > > Review request for Flume. > > > Repository: flume-git > > > Description > ------- > > (from JIRA:) > This Patch changes the configuration of the ElasticSearchClient and also the > ElasticSearchSink. Some parameters that are only relevant for the client are > directly passed to it without using the ElasticSearchClientFactory in > between. The affected tests are changed. > The new feature comes with ElasticSearchClientFactory. It is extended to > create instances of arbitrary FQCNs additionally to rest and transport > clients. There is also a test case for that feature. > Also the way a local transport client for testing is created changed to only > affect the client, but not the sink or the client factory. > > > Diffs > ----- > > flume-ng-doc/sphinx/FlumeUserGuide.rst daf6e72 > > flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java > 1d9dfce > > flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/client/ElasticSearchClient.java > 655e00a > > flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/client/ElasticSearchClientFactory.java > 873157a > > flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/client/ElasticSearchRestClient.java > 0d1c37f > > flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/client/ElasticSearchTransportClient.java > d44c8ad > > flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchSinkTest.java > 48eafdf > > 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/TestElasticSearchSink.java > 15546c1 > > flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TimeBasedIndexNameBuilderTest.java > 678342a > > flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/client/TestElasticSearchClientFactory.java > 4b70b65 > > flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/client/TestElasticSearchRestClient.java > b7d8822 > > flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/client/TestElasticSearchTransportClient.java > b7b8e74 > > Diff: https://reviews.apache.org/r/24213/diff/ > > > Testing > ------- > > > Thanks, > > Bastian Germann > >
