[jira] [Commented] (STORM-1210) Set Output Stream id in KafkaSpout

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/STORM-1210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15081862#comment-15081862
 ] 

ASF GitHub Bot commented on STORM-1210:
---

Github user revans2 commented on the pull request:

https://github.com/apache/storm/pull/885#issuecomment-168817797
  
Still +1


> Set Output Stream id in KafkaSpout
> --
>
> Key: STORM-1210
> URL: https://issues.apache.org/jira/browse/STORM-1210
> Project: Apache Storm
>  Issue Type: New Feature
>  Components: storm-kafka
>Affects Versions: 0.11.0
>Reporter: Zhiqiang He
>Assignee: Zhiqiang He
>Priority: Minor
>
> topicAsStreamId can only set output stream id to topic name. In some case ,we 
> need to set output stream id to other name.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (STORM-1210) Set Output Stream id in KafkaSpout

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/STORM-1210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15081877#comment-15081877
 ] 

ASF GitHub Bot commented on STORM-1210:
---

Github user asfgit closed the pull request at:

https://github.com/apache/storm/pull/885


> Set Output Stream id in KafkaSpout
> --
>
> Key: STORM-1210
> URL: https://issues.apache.org/jira/browse/STORM-1210
> Project: Apache Storm
>  Issue Type: New Feature
>  Components: storm-kafka
>Affects Versions: 0.11.0
>Reporter: Zhiqiang He
>Assignee: Zhiqiang He
>Priority: Minor
> Fix For: 0.11.0
>
>
> topicAsStreamId can only set output stream id to topic name. In some case ,we 
> need to set output stream id to other name.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (STORM-1210) Set Output Stream id in KafkaSpout

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/STORM-1210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15057130#comment-15057130
 ] 

ASF GitHub Bot commented on STORM-1210:
---

Github user rfarivar commented on a diff in the pull request:

https://github.com/apache/storm/pull/885#discussion_r47586077
  
--- Diff: external/storm-kafka/src/jvm/storm/kafka/PartitionManager.java ---
@@ -149,9 +150,9 @@ public EmitState next(SpoutOutputCollector collector) {
 }
 
 if ((tups != null) && tups.iterator().hasNext()) {
-if(_spoutConfig.topicAsStreamId) {
+   if (!Strings.isNullOrEmpty(_spoutConfig.outputStreamId)) {
--- End diff --

Isn't it an overkill to import a whole package 
(com.google.common.base.Strings) to only test for null or empty? Can't we just 
use: 
if (_spoutConfig.outputStreamId!=null && _spoutConfig.outputStreamId!="") 
or something to that effect instead of importing a new library?


> Set Output Stream id in KafkaSpout
> --
>
> Key: STORM-1210
> URL: https://issues.apache.org/jira/browse/STORM-1210
> Project: Apache Storm
>  Issue Type: New Feature
>  Components: storm-kafka
>Affects Versions: 0.11.0
>Reporter: Zhiqiang He
>Assignee: Zhiqiang He
>Priority: Minor
>
> topicAsStreamId can only set output stream id to topic name. In some case ,we 
> need to set output stream id to other name.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (STORM-1210) Set Output Stream id in KafkaSpout

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/STORM-1210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15057161#comment-15057161
 ] 

ASF GitHub Bot commented on STORM-1210:
---

Github user Zhiqiang-He commented on the pull request:

https://github.com/apache/storm/pull/885#issuecomment-164617141
  
@rfarivar Because guava is already import in storm-kafka.
And com.google.common.collect.ImmutableMap is already used in 
PartitionManager.java. 


> Set Output Stream id in KafkaSpout
> --
>
> Key: STORM-1210
> URL: https://issues.apache.org/jira/browse/STORM-1210
> Project: Apache Storm
>  Issue Type: New Feature
>  Components: storm-kafka
>Affects Versions: 0.11.0
>Reporter: Zhiqiang He
>Assignee: Zhiqiang He
>Priority: Minor
>
> topicAsStreamId can only set output stream id to topic name. In some case ,we 
> need to set output stream id to other name.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (STORM-1210) Set Output Stream id in KafkaSpout

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/STORM-1210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15056667#comment-15056667
 ] 

ASF GitHub Bot commented on STORM-1210:
---

Github user ppoulosk commented on the pull request:

https://github.com/apache/storm/pull/885#issuecomment-164551153
  
+1, NB


> Set Output Stream id in KafkaSpout
> --
>
> Key: STORM-1210
> URL: https://issues.apache.org/jira/browse/STORM-1210
> Project: Apache Storm
>  Issue Type: New Feature
>  Components: storm-kafka
>Affects Versions: 0.11.0
>Reporter: Zhiqiang He
>Assignee: Zhiqiang He
>Priority: Minor
>
> topicAsStreamId can only set output stream id to topic name. In some case ,we 
> need to set output stream id to other name.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (STORM-1210) Set Output Stream id in KafkaSpout

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/STORM-1210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15037200#comment-15037200
 ] 

ASF GitHub Bot commented on STORM-1210:
---

Github user wuchong commented on a diff in the pull request:

https://github.com/apache/storm/pull/885#discussion_r46510092
  
--- Diff: external/storm-kafka/src/jvm/storm/kafka/SpoutConfig.java ---
@@ -27,8 +27,7 @@
 public String zkRoot = null;
 public String id = null;
 
-// if set to true, spout will set Kafka topic as the emitted Stream ID
-public boolean topicAsStreamId = false;
+public String outputStreamId;
--- End diff --

Minor, but we can note  here that if outputStreamId not set, spout will set 
Kafka topic as the emitted Stream ID


> Set Output Stream id in KafkaSpout
> --
>
> Key: STORM-1210
> URL: https://issues.apache.org/jira/browse/STORM-1210
> Project: Apache Storm
>  Issue Type: New Feature
>  Components: storm-kafka
>Affects Versions: 0.11.0
>Reporter: Zhiqiang He
>Assignee: Zhiqiang He
>Priority: Minor
>
> topicAsStreamId can only set output stream id to topic name. In some case ,we 
> need to set output stream id to other name.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (STORM-1210) Set Output Stream id in KafkaSpout

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/STORM-1210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15037194#comment-15037194
 ] 

ASF GitHub Bot commented on STORM-1210:
---

Github user Zhiqiang-He commented on the pull request:

https://github.com/apache/storm/pull/885#issuecomment-161506356
  
@revans2 I already removed topicAsStreamId, please review it. thanks.


> Set Output Stream id in KafkaSpout
> --
>
> Key: STORM-1210
> URL: https://issues.apache.org/jira/browse/STORM-1210
> Project: Apache Storm
>  Issue Type: New Feature
>  Components: storm-kafka
>Affects Versions: 0.11.0
>Reporter: Zhiqiang He
>Assignee: Zhiqiang He
>Priority: Minor
>
> topicAsStreamId can only set output stream id to topic name. In some case ,we 
> need to set output stream id to other name.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (STORM-1210) Set Output Stream id in KafkaSpout

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/STORM-1210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15037201#comment-15037201
 ] 

ASF GitHub Bot commented on STORM-1210:
---

Github user wuchong commented on the pull request:

https://github.com/apache/storm/pull/885#issuecomment-161507454
  
+1 Nice work. It is much clearer now.  


> Set Output Stream id in KafkaSpout
> --
>
> Key: STORM-1210
> URL: https://issues.apache.org/jira/browse/STORM-1210
> Project: Apache Storm
>  Issue Type: New Feature
>  Components: storm-kafka
>Affects Versions: 0.11.0
>Reporter: Zhiqiang He
>Assignee: Zhiqiang He
>Priority: Minor
>
> topicAsStreamId can only set output stream id to topic name. In some case ,we 
> need to set output stream id to other name.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (STORM-1210) Set Output Stream id in KafkaSpout

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/STORM-1210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15011747#comment-15011747
 ] 

ASF GitHub Bot commented on STORM-1210:
---

Github user jerrypeng commented on the pull request:

https://github.com/apache/storm/pull/885#issuecomment-157831515
  
just a quick question. where is outputStreamId set?


> Set Output Stream id in KafkaSpout
> --
>
> Key: STORM-1210
> URL: https://issues.apache.org/jira/browse/STORM-1210
> Project: Apache Storm
>  Issue Type: New Feature
>  Components: storm-kafka
>Affects Versions: 0.11.0
>Reporter: Zhiqiang He
>Assignee: Zhiqiang He
>Priority: Minor
>
> topicAsStreamId can only set output stream id to topic name. In some case ,we 
> need to set output stream id to other name.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (STORM-1210) Set Output Stream id in KafkaSpout

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/STORM-1210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15012530#comment-15012530
 ] 

ASF GitHub Bot commented on STORM-1210:
---

Github user Zhiqiang-He commented on the pull request:

https://github.com/apache/storm/pull/885#issuecomment-157915066
  
@jerrypeng 
outputStreamId  is used in declareStream as emit method.
It is very useful when bolt after kafkaspout recive two or more stream 
messages.


> Set Output Stream id in KafkaSpout
> --
>
> Key: STORM-1210
> URL: https://issues.apache.org/jira/browse/STORM-1210
> Project: Apache Storm
>  Issue Type: New Feature
>  Components: storm-kafka
>Affects Versions: 0.11.0
>Reporter: Zhiqiang He
>Assignee: Zhiqiang He
>Priority: Minor
>
> topicAsStreamId can only set output stream id to topic name. In some case ,we 
> need to set output stream id to other name.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (STORM-1210) Set Output Stream id in KafkaSpout

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/STORM-1210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15008514#comment-15008514
 ] 

ASF GitHub Bot commented on STORM-1210:
---

Github user wuchong commented on the pull request:

https://github.com/apache/storm/pull/885#issuecomment-157342872
  
Looks good to me.

But the condition code (`topicAsStreamId` and `outputStreamId`) is a little 
reduplicate as the logic is almost the same.


> Set Output Stream id in KafkaSpout
> --
>
> Key: STORM-1210
> URL: https://issues.apache.org/jira/browse/STORM-1210
> Project: Apache Storm
>  Issue Type: New Feature
>  Components: storm-kafka
>Affects Versions: 0.11.0
>Reporter: Zhiqiang He
>Assignee: Zhiqiang He
>Priority: Minor
>
> topicAsStreamId can only set output stream id to topic name. In some case ,we 
> need to set output stream id to other name.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (STORM-1210) Set Output Stream id in KafkaSpout

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/STORM-1210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15008518#comment-15008518
 ] 

ASF GitHub Bot commented on STORM-1210:
---

Github user Zhiqiang-He commented on the pull request:

https://github.com/apache/storm/pull/885#issuecomment-157344649
  
@wuchong 
Yes, you are right. but for compatibility reasons, I add this argument 
based on topicAsStreamId .



> Set Output Stream id in KafkaSpout
> --
>
> Key: STORM-1210
> URL: https://issues.apache.org/jira/browse/STORM-1210
> Project: Apache Storm
>  Issue Type: New Feature
>  Components: storm-kafka
>Affects Versions: 0.11.0
>Reporter: Zhiqiang He
>Assignee: Zhiqiang He
>Priority: Minor
>
> topicAsStreamId can only set output stream id to topic name. In some case ,we 
> need to set output stream id to other name.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (STORM-1210) Set Output Stream id in KafkaSpout

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/STORM-1210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15008508#comment-15008508
 ] 

ASF GitHub Bot commented on STORM-1210:
---

Github user wuchong commented on the pull request:

https://github.com/apache/storm/pull/885#issuecomment-157341386
  
Looks good to me.

But the condition code is a little reduplicate and verbose , I prefer to 
check only one variable to set streamId with reseting outputStreamId in 
SpoutConfig constructor.

```java
this.outputStreamId = topicAsStreamId ? topic : this.outputStreamId;
```

Not a big deal, just something trivial .


> Set Output Stream id in KafkaSpout
> --
>
> Key: STORM-1210
> URL: https://issues.apache.org/jira/browse/STORM-1210
> Project: Apache Storm
>  Issue Type: New Feature
>  Components: storm-kafka
>Affects Versions: 0.11.0
>Reporter: Zhiqiang He
>Assignee: Zhiqiang He
>Priority: Minor
>
> topicAsStreamId can only set output stream id to topic name. In some case ,we 
> need to set output stream id to other name.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (STORM-1210) Set Output Stream id in KafkaSpout

[Zhiqiang He (JIRA)]

[ 
https://issues.apache.org/jira/browse/STORM-1210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15008348#comment-15008348
 ] 

Zhiqiang He commented on STORM-1210:


https://github.com/apache/storm/pull/885

> Set Output Stream id in KafkaSpout
> --
>
> Key: STORM-1210
> URL: https://issues.apache.org/jira/browse/STORM-1210
> Project: Apache Storm
>  Issue Type: New Feature
>  Components: storm-kafka
>Affects Versions: 0.11.0
>Reporter: Zhiqiang He
>Assignee: Zhiqiang He
>Priority: Minor
>
> topicAsStreamId can only set output stream id to topic name. In some case ,we 
> need to set output stream id to other name.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (STORM-1210) Set Output Stream id in KafkaSpout

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/STORM-1210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15008346#comment-15008346
 ] 

ASF GitHub Bot commented on STORM-1210:
---

GitHub user Zhiqiang-He opened a pull request:

https://github.com/apache/storm/pull/885

[STORM-1210] Set Output Stream id in KafkaSpout

topicAsStreamId can only set output stream id to topic name. In some case 
,we need to set output stream id to other name.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Zhiqiang-He/storm-0914-edit master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/storm/pull/885.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 #885


commit c1a89d9597dfcbfeca260d4264a4b2e374c419be
Author: Zhiqiang He 
Date:   2015-10-21T02:09:12Z

Merge pull request #1 from apache/master

merge 1021

commit 5261160f84bcb0ef73acf4a632d421f7e2ee901d
Author: Zhiqiang He 
Date:   2015-11-17T02:57:09Z

Merge pull request #2 from apache/master

merge 1117

commit 87a4a8b4382de5617d463b23188f18e4f7e7f03c
Author: Zhiqiang-He 
Date:   2015-11-17T09:14:54Z

set output stream id in kafkaSpout




> Set Output Stream id in KafkaSpout
> --
>
> Key: STORM-1210
> URL: https://issues.apache.org/jira/browse/STORM-1210
> Project: Apache Storm
>  Issue Type: New Feature
>  Components: storm-kafka
>Affects Versions: 0.11.0
>Reporter: Zhiqiang He
>Assignee: Zhiqiang He
>Priority: Minor
>
> topicAsStreamId can only set output stream id to topic name. In some case ,we 
> need to set output stream id to other name.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)