> On Aug. 3, 2014, 2:42 p.m., Pawel wrote:
> > flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/client/ElasticSearchClientFactory.java,
> > line 79
> > <https://reviews.apache.org/r/24213/diff/1/?file=649255#file649255line79>
> >
> > Isn't this way to instantiate a class less readable? I think when i
> > future you want to search for class usage you want find it with any tool
> > because of instantiating by class name. You want to be able to instantiate
> > custom client that user can pass in configuration
> > "com.blablabla.NEW_CLIENT_TYPE? ?
>
> Bastian Germann wrote:
> Right, that is the new feature provided by this changeset. It is less
> readable, but it is required for that feature.
Oh sorry, I didn't read the issue title. BTW it is OK for me to have ability to
have custom client types but why also existing clients are instantiated by
name? Can't they be constructed by "new" operator and in other cases there will
be instantiating by class name? Only asking what is a benefit of this solution
over this below:
if (rest) {
new rest
} else if (transport)
new transport
} else {
instantiate by custom name
}
- Pawel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24213/#review49429
-----------------------------------------------------------
On Aug. 3, 2014, 12:22 p.m., Bastian Germann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24213/
> -----------------------------------------------------------
>
> (Updated Aug. 3, 2014, 12:22 p.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
>
>
