[GitHub] storm pull request: STORM-974 Introduces Tuple - ES document mapp...
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/679 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-974 Introduces Tuple - ES document mapp...
Github user harshach commented on the pull request: https://github.com/apache/storm/pull/679#issuecomment-135289297 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-974 Introduces Tuple - ES document mapp...
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/679#issuecomment-131982590 @harshach Could you take a look? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-974 Introduces Tuple - ES document mapp...
Github user sweetest commented on the pull request: https://github.com/apache/storm/pull/679#issuecomment-131676548 LGTM --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-974 Introduces Tuple - ES document mapp...
Github user sweetest commented on a diff in the pull request:
https://github.com/apache/storm/pull/679#discussion_r37155617
--- Diff:
external/storm-elasticsearch/src/test/java/org/apache/storm/elasticsearch/common/EsTestUtil.java
---
@@ -46,6 +47,30 @@ public Fields getComponentOutputFields(String
componentId, String streamId) {
return new TupleImpl(topologyContext, new Values(source, index,
type, id), 1, );
}
+public static EsTupleMapper generateDefaultTupleMapper() {
+return new EsTupleMapper() {
+@Override
+public String getSource(ITuple tuple) {
+return tuple.getStringByField(source);
+}
+
+@Override
+public String getIndex(ITuple tuple) {
+return tuple.getStringByField(index);
+}
+
+@Override
+public String getType(ITuple tuple) {
+return tuple.getStringByField(type);
+}
+
+@Override
+public String getId(ITuple tuple) {
+return tuple.getStringByField(id);
+}
+};
+}
+
--- End diff --
can't we just use SampleEsTupleMapper instance instead?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-974 Introduces Tuple - ES document mapp...
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/679#discussion_r37156295
--- Diff:
external/storm-elasticsearch/src/test/java/org/apache/storm/elasticsearch/common/EsTestUtil.java
---
@@ -46,6 +47,30 @@ public Fields getComponentOutputFields(String
componentId, String streamId) {
return new TupleImpl(topologyContext, new Values(source, index,
type, id), 1, );
}
+public static EsTupleMapper generateDefaultTupleMapper() {
+return new EsTupleMapper() {
+@Override
+public String getSource(ITuple tuple) {
+return tuple.getStringByField(source);
+}
+
+@Override
+public String getIndex(ITuple tuple) {
+return tuple.getStringByField(index);
+}
+
+@Override
+public String getType(ITuple tuple) {
+return tuple.getStringByField(type);
+}
+
+@Override
+public String getId(ITuple tuple) {
+return tuple.getStringByField(id);
+}
+};
+}
+
--- End diff --
@sweetest
SampleEsTupleMapper is just a class I defined from README.
But I also think it would be better to have default mapper, and its name
would be DefaultEsTupleMapper.
I'll reflect it. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-974 Introduces Tuple - ES document mapp...
Github user sweetest commented on a diff in the pull request:
https://github.com/apache/storm/pull/679#discussion_r37155605
--- Diff: external/storm-elasticsearch/README.md ---
@@ -6,37 +6,75 @@
## EsIndexBolt (org.apache.storm.elasticsearch.bolt.EsIndexBolt)
EsIndexBolt streams tuples directly into Elasticsearch. Tuples are indexed
in specified index type combination.
-User should make sure that there are source, index,type, and id
fields declared in preceding bolts or spout.
-index and type fields are used for identifying target index and type.
+Users should make sure that ```EsTupleMapper``` can extract source,
index, type, and id from input tuple.
+index and type are used for identifying target index and type.
source is a document in JSON format string that will be indexed in
Elasticsearch.
```java
+class SampleEsTupleMapper implements EsTupleMapper {
+@Override
+public String getSource(ITuple tuple) {
+return tuple.getStringByField(source);
+}
+
+@Override
+public String getIndex(ITuple tuple) {
+return tuple.getStringByField(index);
+}
+
+@Override
+public String getType(ITuple tuple) {
+return tuple.getStringByField(type);
+}
+
+@Override
+public String getId(ITuple tuple) {
+return tuple.getStringByField(id);
+}
+}
+
--- End diff --
How about naming this module as DefaultEsTupleMapper?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-974 Introduces Tuple - ES document mapp...
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/679#issuecomment-131676338 @sweetest Addressed your comments. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-974 Introduces Tuple - ES document mapp...
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/679#issuecomment-130574215 @caofangkun Thanks for the review. I removed unused fields / imports. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-974 Introduces Tuple - ES document mapp...
Github user caofangkun commented on a diff in the pull request: https://github.com/apache/storm/pull/679#discussion_r36938144 --- Diff: external/storm-elasticsearch/src/main/java/org/apache/storm/elasticsearch/trident/EsStateFactory.java --- @@ -19,6 +19,7 @@ import backtype.storm.task.IMetricsContext; import org.apache.storm.elasticsearch.common.EsConfig; +import org.apache.storm.elasticsearch.common.EsTupleMapper; import org.slf4j.Logger; import org.slf4j.LoggerFactory; --- End diff -- this two line slf4j imports could be removed too --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-974 Introduces Tuple - ES document mapp...
Github user caofangkun commented on a diff in the pull request: https://github.com/apache/storm/pull/679#discussion_r36938292 --- Diff: external/storm-elasticsearch/src/test/java/org/apache/storm/elasticsearch/bolt/EsPercolateBoltTest.java --- @@ -21,6 +21,7 @@ import backtype.storm.tuple.Values; import org.apache.storm.elasticsearch.common.EsConfig; import org.apache.storm.elasticsearch.common.EsTestUtil; +import org.apache.storm.elasticsearch.common.EsTupleMapper; import org.elasticsearch.action.count.CountResponse; import org.elasticsearch.action.percolate.PercolateResponse; import org.elasticsearch.index.query.TermQueryBuilder; --- End diff -- Line 25 27 28 unneeded imports --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-974 Introduces Tuple - ES document mapp...
Github user caofangkun commented on a diff in the pull request: https://github.com/apache/storm/pull/679#discussion_r36938034 --- Diff: external/storm-elasticsearch/src/main/java/org/apache/storm/elasticsearch/bolt/EsPercolateBolt.java --- @@ -31,14 +32,21 @@ --- End diff -- Trivial, this two line imports could be removed --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-974 Introduces Tuple - ES document mapp...
Github user caofangkun commented on a diff in the pull request: https://github.com/apache/storm/pull/679#discussion_r36938233 --- Diff: external/storm-elasticsearch/src/test/java/org/apache/storm/elasticsearch/bolt/EsIndexBoltTest.java --- @@ -20,6 +20,7 @@ import backtype.storm.tuple.Tuple; import org.apache.storm.elasticsearch.common.EsConfig; import org.apache.storm.elasticsearch.common.EsTestUtil; +import org.apache.storm.elasticsearch.common.EsTupleMapper; import org.elasticsearch.action.count.CountRequest; import org.elasticsearch.action.count.CountRequestBuilder; import org.elasticsearch.action.count.CountResponse; --- End diff -- Line 24 25 27 are unnecessary imports too --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-974 Introduces Tuple - ES document mapp...
GitHub user HeartSaVioR opened a pull request: https://github.com/apache/storm/pull/679 STORM-974 Introduces Tuple - ES document mapper to get rid of constant field mapping * EsTupleMapper defines how to extract source, index, type, and id from tuple for ElasticSearch. * Applies EsTupleMapper to completely get rid of constant field mapping. * Also modifying README.md to show how to use. You can merge this pull request into a Git repository by running: $ git pull https://github.com/HeartSaVioR/storm STORM-974 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/679.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #679 commit 1f93a3f0f8194715c35869766dec5fc623066d3e Author: Jungtaek Lim kabh...@gmail.com Date: 2015-08-12T13:46:13Z STORM-974 Introduces Tuple - ES document mapper to get rid of constant field mapping * EsTupleMapper defines how to extract source, index, type, and id from tuple for ElasticSearch. * Applies EsTupleMapper to completely get rid of constant field mapping. * Also modifying README.md to show how to use. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
