You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by anishek <gi...@git.apache.org> on 2015/06/23 06:19:07 UTC
[GitHub] storm pull request: Ability to emit from the spout to a specific s...
GitHub user anishek opened a pull request:
https://github.com/apache/storm/pull/601
Ability to emit from the spout to a specific stream, by default it will emit to the default stream.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/anishek/storm master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/601.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 #601
----
commit 0d1e40bfdd828d32d9e9af7618209aeebb37a1fa
Author: Anishek Agarwal <an...@adnear.com>
Date: 2015-06-23T04:18:09Z
Ability to emit from the spout to a specific stream, by default it will emit to the default stream.
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: Ability to emit from the spout to a specific s...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/601#issuecomment-115952938
And please modify PR title to include "STORM-917" as prefix.
Thanks in advance!
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-917 : Ability to emit from the spout to ...
Posted by anishek <gi...@git.apache.org>.
Github user anishek commented on a diff in the pull request:
https://github.com/apache/storm/pull/601#discussion_r33414016
--- Diff: external/storm-kafka/src/jvm/storm/kafka/SpoutConfig.java ---
@@ -29,6 +29,7 @@
// setting for how often to save the current kafka offset to ZooKeeper
public long stateUpdateIntervalMs = 2000;
+ public String emitStreamId;
--- End diff --
you are right i didnt look at the code too closely, I had a overridden method in PartitionManager with the default stream. I thought anyone else using the old method would automatically go through that. Though thinking now i looked at all the usages and there is only one in the kafkaSpout so i will do another pull request with the changes.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-917 : Ability to emit from the spout to ...
Posted by anishek <gi...@git.apache.org>.
Github user anishek commented on the pull request:
https://github.com/apache/storm/pull/601#issuecomment-115956503
done hope this helps.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-917 : Ability to emit from the spout to ...
Posted by anishek <gi...@git.apache.org>.
Github user anishek commented on the pull request:
https://github.com/apache/storm/pull/601#issuecomment-187051503
@lujinhong - i have not worked with trident and hence don't know where to make the change.
@abhishekagarwal87 -- this was a quick fix i did for a POC under 30 mins, It might not be the right way to go about this.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-917 : Ability to emit from the spout to ...
Posted by anishek <gi...@git.apache.org>.
Github user anishek commented on the pull request:
https://github.com/apache/storm/pull/601#issuecomment-116737782
@harshach I did add the line with the comment in the https://github.com/apache/storm/blob/master/external/storm-kafka/README.md stating what it does under the set of configurations for the spoutConfig. Do you want a separate section or a separate line in the read me explaining it ?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-917 : Ability to emit from the spout to ...
Posted by anishek <gi...@git.apache.org>.
Github user anishek commented on a diff in the pull request:
https://github.com/apache/storm/pull/601#discussion_r53592727
--- Diff: external/storm-kafka/src/jvm/storm/kafka/SpoutConfig.java ---
@@ -17,6 +17,8 @@
*/
package storm.kafka;
+import backtype.storm.utils.Utils;
--- End diff --
Upmerge ? i thought one of you would pull in the request and merge it.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: Ability to emit from the spout to a specific s...
Posted by anishek <gi...@git.apache.org>.
Github user anishek commented on the pull request:
https://github.com/apache/storm/pull/601#issuecomment-115952859
https://issues.apache.org/jira/browse/STORM-917
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-917 : Ability to emit from the spout to ...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/601#discussion_r33413898
--- Diff: external/storm-kafka/src/jvm/storm/kafka/SpoutConfig.java ---
@@ -29,6 +29,7 @@
// setting for how often to save the current kafka offset to ZooKeeper
public long stateUpdateIntervalMs = 2000;
+ public String emitStreamId;
--- End diff --
is it a mandatory config now? . What if users doesn't pass a emitStreamId shouldn't we use the default.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-917 : Ability to emit from the spout to ...
Posted by sweetest <gi...@git.apache.org>.
Github user sweetest commented on the pull request:
https://github.com/apache/storm/pull/601#issuecomment-116366095
This is a gread idea, though I haven't looked through entire code.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-917 : Ability to emit from the spout to ...
Posted by abhishekagarwal87 <gi...@git.apache.org>.
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/601#discussion_r53569055
--- Diff: external/storm-kafka/src/jvm/storm/kafka/PartitionManager.java ---
@@ -126,7 +127,7 @@ public Map getMetricsDataMap() {
}
//returns false if it's reached the end of current batch
- public EmitState next(SpoutOutputCollector collector) {
+ public EmitState next(SpoutOutputCollector collector, String emitStreamId) {
--- End diff --
what is the need of additional argument since spoutConfig is available to this class. stream name can be initialized once in the prepare phase itself.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-917 : Ability to emit from the spout to ...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the pull request:
https://github.com/apache/storm/pull/601#issuecomment-116895161
@anishek my bad missed it.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-917 : Ability to emit from the spout to ...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the pull request:
https://github.com/apache/storm/pull/601#issuecomment-116731268
@anishek can you please add this config to README.md https://github.com/apache/storm/blob/master/external/storm-kafka/README.md
few lines about what it does.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-917 : Ability to emit from the spout to ...
Posted by abhishekagarwal87 <gi...@git.apache.org>.
Github user abhishekagarwal87 commented on the pull request:
https://github.com/apache/storm/pull/601#issuecomment-186855234
Thanks for doing this Anishek. My comments
Kafka spout should declare the stream id in output declaration.
```
declarer.declare(streamID, _spoutConfig.scheme.getOutputFields()
```
I would prefer the streamID to come from the scheme. More than often the stream on which data is to be sent to, depends on the input data. Since scheme knows about the data, it can also determine the stream. This makes more sense when you think of multiple output streams from same spout. The only downside I see is that the scheme interface will change and that might translate into compatibility issues. Nonetheless this PR is a good start.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-917 : Ability to emit from the spout to ...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the pull request:
https://github.com/apache/storm/pull/601#issuecomment-116099001
@anishek can you also update the README.md with doc on this config.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-917 : Ability to emit from the spout to ...
Posted by abhishekagarwal87 <gi...@git.apache.org>.
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/601#discussion_r53569108
--- Diff: external/storm-kafka/src/jvm/storm/kafka/SpoutConfig.java ---
@@ -17,6 +17,8 @@
*/
package storm.kafka;
+import backtype.storm.utils.Utils;
--- End diff --
You need to upmerge your master branch.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-917 : Ability to emit from the spout to ...
Posted by abhishekagarwal87 <gi...@git.apache.org>.
Github user abhishekagarwal87 commented on a diff in the pull request:
https://github.com/apache/storm/pull/601#discussion_r53593326
--- Diff: external/storm-kafka/src/jvm/storm/kafka/SpoutConfig.java ---
@@ -17,6 +17,8 @@
*/
package storm.kafka;
+import backtype.storm.utils.Utils;
--- End diff --
One of the committers will do that. But for that to happen, the PR should be ready for merge. Currently it has conflicts with master branch.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: Ability to emit from the spout to a specific s...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/601#issuecomment-115923850
I like the idea.
Could you file issue to JIRA so that we can record changeset into CHANGELOG?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-917 : Ability to emit from the spout to ...
Posted by lujinhong <gi...@git.apache.org>.
Github user lujinhong commented on the pull request:
https://github.com/apache/storm/pull/601#issuecomment-186567237
what about trident?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-917 : Ability to emit from the spout to ...
Posted by anishek <gi...@git.apache.org>.
Github user anishek commented on the pull request:
https://github.com/apache/storm/pull/601#issuecomment-116428361
All the necessary changes are in this request. Please do let me know if anything else is required.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #601: STORM-917 : Ability to emit from the spout to a specific s...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the issue:
https://github.com/apache/storm/pull/601
> this was a quick fix i did for a POC under 30 mins, It might not be the right way to go about this.
@anishek This pull request has gone stale. Do we want to continue with this proposed change or close this pull request?
---
[GitHub] storm pull request #601: STORM-917 : Ability to emit from the spout to a spe...
Posted by anishek <gi...@git.apache.org>.
Github user anishek closed the pull request at:
https://github.com/apache/storm/pull/601
---