You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Edward Sargisson <es...@pobox.com> on 2013/04/09 19:59:22 UTC
Review Request: FLUME-1972 ElasticSearchSink now allows the id of the
document to be provided by a custom class
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10380/
-----------------------------------------------------------
Review request for Flume and Hari Shreedharan.
Description
-------
Elasticsearch allows clients to specify the id of the newly added document - and will generate one by default if none is supplied. If the user knows that the incoming event has a unique id then that id can be used as the elasticsearch document id. This change enhances the ElasticSearchSink to allow users to provide a custom class to decide what the id should be.
This addresses bug FLUME-1972.
https://issues.apache.org/jira/browse/FLUME-1972
Diffs
-----
flume-ng-doc/sphinx/FlumeUserGuide.rst 693c0d7
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchIdProvider.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchNullIdProvider.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSinkConstants.java 7f75e22
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchSinkTest.java 2edacdc
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
Diff: https://reviews.apache.org/r/10380/diff/
Testing
-------
All unit tests and integration tests pass. A snapshot using commit 5b9d31f1ad228 and the patch for flume-1972 has passed our internal integration tests using customisations.
Thanks,
Edward Sargisson
Re: Review Request: FLUME-1972 ElasticSearchSink now allows the id of the
document to be provided by a custom class
Posted by Israel Ekpo <is...@aicer.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10380/#review18944
-----------------------------------------------------------
Mr. Sargisson,
Thanks for posting this review.
>From an elasticsearch HTTP client perspective, the patch looks good.
However, in your patch, the only class that implements ElasticSearchIdProvider is ElasticSearchNullIdProvider and it returns a null id which means ES is still going to have to generate one when the document is indexed.
Based on the description of the patch reviewed, it would have been nice to see another example class that implements ElasticSearchIdProvider that actually generates a non-null document id so that this default behavior can be overriden.
So maybe generating a document id based on the original timestamp in the event optionally in combination of other headers would be nice.
Nevertheless, this is a good effort.
Thanks.
- Israel Ekpo
On April 9, 2013, 5:59 p.m., Edward Sargisson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10380/
> -----------------------------------------------------------
>
> (Updated April 9, 2013, 5:59 p.m.)
>
>
> Review request for Flume and Hari Shreedharan.
>
>
> Description
> -------
>
> Elasticsearch allows clients to specify the id of the newly added document - and will generate one by default if none is supplied. If the user knows that the incoming event has a unique id then that id can be used as the elasticsearch document id. This change enhances the ElasticSearchSink to allow users to provide a custom class to decide what the id should be.
>
>
> This addresses bug FLUME-1972.
> https://issues.apache.org/jira/browse/FLUME-1972
>
>
> Diffs
> -----
>
> flume-ng-doc/sphinx/FlumeUserGuide.rst 693c0d7
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchIdProvider.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchNullIdProvider.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSinkConstants.java 7f75e22
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchSinkTest.java 2edacdc
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
>
> Diff: https://reviews.apache.org/r/10380/diff/
>
>
> Testing
> -------
>
> All unit tests and integration tests pass. A snapshot using commit 5b9d31f1ad228 and the patch for flume-1972 has passed our internal integration tests using customisations.
>
>
> Thanks,
>
> Edward Sargisson
>
>
Re: Review Request: FLUME-1972 ElasticSearchSink now allows the id of the
document to be provided by a custom class
Posted by Israel Ekpo <is...@aicer.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10380/#review19031
-----------------------------------------------------------
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchExtractIdFromHeaderIdProvider.java
<https://reviews.apache.org/r/10380/#comment39554>
This is much better.
The class name is self descriptive and it confirms to the rest of the pattern.
- Israel Ekpo
On April 11, 2013, 7:02 p.m., Edward Sargisson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10380/
> -----------------------------------------------------------
>
> (Updated April 11, 2013, 7:02 p.m.)
>
>
> Review request for Flume and Hari Shreedharan.
>
>
> Description
> -------
>
> Elasticsearch allows clients to specify the id of the newly added document - and will generate one by default if none is supplied. If the user knows that the incoming event has a unique id then that id can be used as the elasticsearch document id. This change enhances the ElasticSearchSink to allow users to provide a custom class to decide what the id should be.
>
>
> This addresses bug FLUME-1972.
> https://issues.apache.org/jira/browse/FLUME-1972
>
>
> Diffs
> -----
>
> flume-ng-doc/sphinx/FlumeUserGuide.rst 693c0d7
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchExtractIdFromHeaderIdProvider.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchIdProvider.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchNullIdProvider.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSinkConstants.java 7f75e22
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchSinkTest.java 2edacdc
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/ElasticSearchExtractIdFromHeaderIdProviderTest.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
>
> Diff: https://reviews.apache.org/r/10380/diff/
>
>
> Testing
> -------
>
> All unit tests and integration tests pass. A snapshot using commit 5b9d31f1ad228 and the patch for flume-1972 has passed our internal integration tests using customisations.
>
>
> Thanks,
>
> Edward Sargisson
>
>
Re: Review Request: FLUME-1972 ElasticSearchSink now allows the id of the
document to be provided by a custom class
Posted by Israel Ekpo <is...@aicer.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10380/#review19029
-----------------------------------------------------------
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java
<https://reviews.apache.org/r/10380/#comment39545>
That's better.
- Israel Ekpo
On April 11, 2013, 7:02 p.m., Edward Sargisson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10380/
> -----------------------------------------------------------
>
> (Updated April 11, 2013, 7:02 p.m.)
>
>
> Review request for Flume and Hari Shreedharan.
>
>
> Description
> -------
>
> Elasticsearch allows clients to specify the id of the newly added document - and will generate one by default if none is supplied. If the user knows that the incoming event has a unique id then that id can be used as the elasticsearch document id. This change enhances the ElasticSearchSink to allow users to provide a custom class to decide what the id should be.
>
>
> This addresses bug FLUME-1972.
> https://issues.apache.org/jira/browse/FLUME-1972
>
>
> Diffs
> -----
>
> flume-ng-doc/sphinx/FlumeUserGuide.rst 693c0d7
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchExtractIdFromHeaderIdProvider.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchIdProvider.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchNullIdProvider.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSinkConstants.java 7f75e22
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchSinkTest.java 2edacdc
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/ElasticSearchExtractIdFromHeaderIdProviderTest.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
>
> Diff: https://reviews.apache.org/r/10380/diff/
>
>
> Testing
> -------
>
> All unit tests and integration tests pass. A snapshot using commit 5b9d31f1ad228 and the patch for flume-1972 has passed our internal integration tests using customisations.
>
>
> Thanks,
>
> Edward Sargisson
>
>
Re: Review Request: FLUME-1972 ElasticSearchSink now allows the id of the
document to be provided by a custom class
Posted by Israel Ekpo <is...@aicer.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10380/#review19033
-----------------------------------------------------------
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java
<https://reviews.apache.org/r/10380/#comment39555>
If an exception is thrown in 332, then there would be no need to instantiated the object in 326.
- Israel Ekpo
On April 11, 2013, 7:02 p.m., Edward Sargisson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10380/
> -----------------------------------------------------------
>
> (Updated April 11, 2013, 7:02 p.m.)
>
>
> Review request for Flume and Hari Shreedharan.
>
>
> Description
> -------
>
> Elasticsearch allows clients to specify the id of the newly added document - and will generate one by default if none is supplied. If the user knows that the incoming event has a unique id then that id can be used as the elasticsearch document id. This change enhances the ElasticSearchSink to allow users to provide a custom class to decide what the id should be.
>
>
> This addresses bug FLUME-1972.
> https://issues.apache.org/jira/browse/FLUME-1972
>
>
> Diffs
> -----
>
> flume-ng-doc/sphinx/FlumeUserGuide.rst 693c0d7
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchExtractIdFromHeaderIdProvider.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchIdProvider.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchNullIdProvider.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSinkConstants.java 7f75e22
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchSinkTest.java 2edacdc
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/ElasticSearchExtractIdFromHeaderIdProviderTest.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
>
> Diff: https://reviews.apache.org/r/10380/diff/
>
>
> Testing
> -------
>
> All unit tests and integration tests pass. A snapshot using commit 5b9d31f1ad228 and the patch for flume-1972 has passed our internal integration tests using customisations.
>
>
> Thanks,
>
> Edward Sargisson
>
>
Re: Review Request: FLUME-1972 ElasticSearchSink now allows the id of the
document to be provided by a custom class
Posted by Edward Sargisson <es...@pobox.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10380/
-----------------------------------------------------------
(Updated April 18, 2013, 11:56 p.m.)
Review request for Flume and Hari Shreedharan.
Changes
-------
Removed unnecessary instantiation.
Description
-------
Elasticsearch allows clients to specify the id of the newly added document - and will generate one by default if none is supplied. If the user knows that the incoming event has a unique id then that id can be used as the elasticsearch document id. This change enhances the ElasticSearchSink to allow users to provide a custom class to decide what the id should be.
This addresses bug FLUME-1972.
https://issues.apache.org/jira/browse/FLUME-1972
Diffs (updated)
-----
flume-ng-doc/sphinx/FlumeUserGuide.rst 38f2205
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchExtractIdFromHeaderIdProvider.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchIdProvider.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchNullIdProvider.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSinkConstants.java 7f75e22
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchSinkTest.java 2edacdc
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/ElasticSearchExtractIdFromHeaderIdProviderTest.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
Diff: https://reviews.apache.org/r/10380/diff/
Testing
-------
All unit tests and integration tests pass. A snapshot using commit 5b9d31f1ad228 and the patch for flume-1972 has passed our internal integration tests using customisations.
Thanks,
Edward Sargisson
Re: Review Request: FLUME-1972 ElasticSearchSink now allows the id of the
document to be provided by a custom class
Posted by Edward Sargisson <es...@pobox.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10380/
-----------------------------------------------------------
(Updated April 11, 2013, 7:02 p.m.)
Review request for Flume and Hari Shreedharan.
Changes
-------
* Verify that the custom id provider class exists and is assignable to the ElasticsearchIdProvider interface.
* Renamed the example class to ElasticSearchExtractIdFromHeaderIdProvider
Description
-------
Elasticsearch allows clients to specify the id of the newly added document - and will generate one by default if none is supplied. If the user knows that the incoming event has a unique id then that id can be used as the elasticsearch document id. This change enhances the ElasticSearchSink to allow users to provide a custom class to decide what the id should be.
This addresses bug FLUME-1972.
https://issues.apache.org/jira/browse/FLUME-1972
Diffs (updated)
-----
flume-ng-doc/sphinx/FlumeUserGuide.rst 693c0d7
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchExtractIdFromHeaderIdProvider.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchIdProvider.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchNullIdProvider.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSinkConstants.java 7f75e22
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchSinkTest.java 2edacdc
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/ElasticSearchExtractIdFromHeaderIdProviderTest.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
Diff: https://reviews.apache.org/r/10380/diff/
Testing
-------
All unit tests and integration tests pass. A snapshot using commit 5b9d31f1ad228 and the patch for flume-1972 has passed our internal integration tests using customisations.
Thanks,
Edward Sargisson
Re: Review Request: FLUME-1972 ElasticSearchSink now allows the id of the
document to be provided by a custom class
Posted by Israel Ekpo <is...@aicer.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10380/#review18963
-----------------------------------------------------------
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java
<https://reviews.apache.org/r/10380/#comment39423>
Sometimes, typos do occur in the configuration files for these custom class names.
I think it would be nice to see if clazz is not null and also assignable before we try to instantiate an object for the provider.
You should also log that the code was not able to create an instance from class specified in the config file.
This will make things easier to troubleshoot when things go wrong.
Take a look at an example here for some tips:
org.apache.flume.serialization.EventSerializerFactory
- Israel Ekpo
On April 10, 2013, 5 p.m., Edward Sargisson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10380/
> -----------------------------------------------------------
>
> (Updated April 10, 2013, 5 p.m.)
>
>
> Review request for Flume and Hari Shreedharan.
>
>
> Description
> -------
>
> Elasticsearch allows clients to specify the id of the newly added document - and will generate one by default if none is supplied. If the user knows that the incoming event has a unique id then that id can be used as the elasticsearch document id. This change enhances the ElasticSearchSink to allow users to provide a custom class to decide what the id should be.
>
>
> This addresses bug FLUME-1972.
> https://issues.apache.org/jira/browse/FLUME-1972
>
>
> Diffs
> -----
>
> flume-ng-doc/sphinx/FlumeUserGuide.rst 693c0d7
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchIdProvider.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchNullIdProvider.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSinkConstants.java 7f75e22
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/IdHeaderElasticSearchIdProvider.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchSinkTest.java 2edacdc
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/IdHeaderElasticSearchIdProviderTest.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
>
> Diff: https://reviews.apache.org/r/10380/diff/
>
>
> Testing
> -------
>
> All unit tests and integration tests pass. A snapshot using commit 5b9d31f1ad228 and the patch for flume-1972 has passed our internal integration tests using customisations.
>
>
> Thanks,
>
> Edward Sargisson
>
>
Re: Review Request: FLUME-1972 ElasticSearchSink now allows the id of the
document to be provided by a custom class
Posted by Israel Ekpo <is...@aicer.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10380/#review18996
-----------------------------------------------------------
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/IdHeaderElasticSearchIdProvider.java
<https://reviews.apache.org/r/10380/#comment39477>
I would prefer if the class names for the document id providers are consistent.
Consider renaming this class to something like ElasticSearchExtractIdFromHeaderIdProvider or similar.
- Israel Ekpo
On April 10, 2013, 5 p.m., Edward Sargisson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10380/
> -----------------------------------------------------------
>
> (Updated April 10, 2013, 5 p.m.)
>
>
> Review request for Flume and Hari Shreedharan.
>
>
> Description
> -------
>
> Elasticsearch allows clients to specify the id of the newly added document - and will generate one by default if none is supplied. If the user knows that the incoming event has a unique id then that id can be used as the elasticsearch document id. This change enhances the ElasticSearchSink to allow users to provide a custom class to decide what the id should be.
>
>
> This addresses bug FLUME-1972.
> https://issues.apache.org/jira/browse/FLUME-1972
>
>
> Diffs
> -----
>
> flume-ng-doc/sphinx/FlumeUserGuide.rst 693c0d7
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchIdProvider.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchNullIdProvider.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSinkConstants.java 7f75e22
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/IdHeaderElasticSearchIdProvider.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchSinkTest.java 2edacdc
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/IdHeaderElasticSearchIdProviderTest.java PRE-CREATION
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
>
> Diff: https://reviews.apache.org/r/10380/diff/
>
>
> Testing
> -------
>
> All unit tests and integration tests pass. A snapshot using commit 5b9d31f1ad228 and the patch for flume-1972 has passed our internal integration tests using customisations.
>
>
> Thanks,
>
> Edward Sargisson
>
>
Re: Review Request: FLUME-1972 ElasticSearchSink now allows the id of the
document to be provided by a custom class
Posted by Edward Sargisson <es...@pobox.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10380/
-----------------------------------------------------------
(Updated April 10, 2013, 5 p.m.)
Review request for Flume and Hari Shreedharan.
Changes
-------
Mr Ekpo,
Thank you for your comments.
As a good example, how about I provide the custom implementation we are currently using which reads the id header and uses that as the document id?
I have updated the patch to include it.
Cheers,
Edward
Description
-------
Elasticsearch allows clients to specify the id of the newly added document - and will generate one by default if none is supplied. If the user knows that the incoming event has a unique id then that id can be used as the elasticsearch document id. This change enhances the ElasticSearchSink to allow users to provide a custom class to decide what the id should be.
This addresses bug FLUME-1972.
https://issues.apache.org/jira/browse/FLUME-1972
Diffs (updated)
-----
flume-ng-doc/sphinx/FlumeUserGuide.rst 693c0d7
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchIdProvider.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchNullIdProvider.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java 1b3db14
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSinkConstants.java 7f75e22
flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/IdHeaderElasticSearchIdProvider.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchSinkTest.java 2edacdc
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/IdHeaderElasticSearchIdProviderTest.java PRE-CREATION
flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java 94b95b1
Diff: https://reviews.apache.org/r/10380/diff/
Testing
-------
All unit tests and integration tests pass. A snapshot using commit 5b9d31f1ad228 and the patch for flume-1972 has passed our internal integration tests using customisations.
Thanks,
Edward Sargisson