Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/254#discussion_r32229690
--- Diff:
external/storm-kafka/src/jvm/storm/kafka/ExponentialBackoffMsgRetryManager.java
---
@@ -0,0 +1,167 @@
+/**
+ * Licensed to the Apache Software F
Github user kenshih commented on a diff in the pull request:
https://github.com/apache/storm/pull/254#discussion_r24383036
--- Diff:
external/storm-kafka/src/jvm/storm/kafka/ExponentialBackoffMsgRetryManager.java
---
@@ -0,0 +1,167 @@
+/**
+ * Licensed to the Apache Softwa
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/254
---
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 enabl
Github user rick-kilgore commented on the pull request:
https://github.com/apache/storm/pull/254#issuecomment-69615838
What is the next step on this PR? Should I merge from master?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/254#issuecomment-69598121
+1 the new changes look fine to me. I am not so sure about HBOCodeLabs#4
It should be fine, I'm just not super thrilled about changing static methods to
be instance meth
Github user rick-kilgore commented on the pull request:
https://github.com/apache/storm/pull/254#issuecomment-69433393
@revans2 I made changes to address both your comments. Please have a look.
That's a good catch about how I was using Comparable - thanks!
By the way,
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/254#issuecomment-69242628
@rick-kilgore I did a pass through the code and it looks good to me. I
have two comments that I see as nice to have, but I don't think would block
this from going in. I'
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/254#discussion_r22678403
--- Diff:
external/storm-kafka/src/jvm/storm/kafka/ExponentialBackoffMsgRetryManager.java
---
@@ -0,0 +1,154 @@
+/**
+ * Licensed to the Apache Softwa
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/254#discussion_r22678017
--- Diff:
external/storm-kafka/src/jvm/storm/kafka/ExponentialBackoffMsgRetryManager.java
---
@@ -0,0 +1,154 @@
+/**
+ * Licensed to the Apache Softwa
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/254#issuecomment-66837722
+1 (again). Looks good to me.
---
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
Github user rick-kilgore commented on the pull request:
https://github.com/apache/storm/pull/254#issuecomment-64317893
This PR is now update to incorporate most of the changes suggested by
@wurstmeister. I also discovered that the conflict I was worried about with
TOPOLOGY_MESSAGE_TI
Github user rick-kilgore commented on the pull request:
https://github.com/apache/storm/pull/254#issuecomment-60354346
I should be able to find time to add the additional encapsulation in my
solution and update it with changes in master over this weekend if you guys
agree that's a goo
Github user rick-kilgore commented on the pull request:
https://github.com/apache/storm/pull/254#issuecomment-60354121
Hi @wurstmeister. I like the way yours encapsulates the collections into
the separate FailedMessageHandler class. That's definitely cleaner.
I have two comments:
Github user wurstmeister commented on the pull request:
https://github.com/apache/storm/pull/254#issuecomment-58272817
Hi,
sorry Iâm a it late to this topic.
I do think this is something that should be pluggable (and testable).
I have prepared a small *ske
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/254#issuecomment-58241961
+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 w
Github user rick-kilgore commented on a diff in the pull request:
https://github.com/apache/storm/pull/254#discussion_r18382648
--- Diff: external/storm-kafka/src/jvm/storm/kafka/SpoutConfig.java ---
@@ -26,8 +26,19 @@
public Integer zkPort = null;
public String zk
Github user rick-kilgore commented on a diff in the pull request:
https://github.com/apache/storm/pull/254#discussion_r18353211
--- Diff: external/storm-kafka/src/jvm/storm/kafka/SpoutConfig.java ---
@@ -26,8 +26,19 @@
public Integer zkPort = null;
public String zk
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/254#discussion_r18337515
--- Diff: external/storm-kafka/src/jvm/storm/kafka/SpoutConfig.java ---
@@ -26,8 +26,19 @@
public Integer zkPort = null;
public String zkRoot = nu
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/254#discussion_r18323164
--- Diff: external/storm-kafka/src/jvm/storm/kafka/SpoutConfig.java ---
@@ -26,8 +26,19 @@
public Integer zkPort = null;
public String zkRoot
Github user rick-kilgore commented on a diff in the pull request:
https://github.com/apache/storm/pull/254#discussion_r18317774
--- Diff: external/storm-kafka/src/jvm/storm/kafka/SpoutConfig.java ---
@@ -26,8 +26,19 @@
public Integer zkPort = null;
public String zk
Github user rick-kilgore closed the pull request at:
https://github.com/apache/storm/pull/276
---
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
Github user rick-kilgore commented on the pull request:
https://github.com/apache/storm/pull/276#issuecomment-57561566
don't need this one - instead use https://github.com/apache/storm/pull/254
---
If your project is set up for it, you can reply to this email and have your
reply appea
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/254#issuecomment-57561410
@rick-kilgore You can go ahead and close the other pull request.
Also see my comment regarding throwing an exception if the configuration is
out of whack. Once tha
Github user ptgoetz commented on a diff in the pull request:
https://github.com/apache/storm/pull/254#discussion_r18316629
--- Diff: external/storm-kafka/src/jvm/storm/kafka/SpoutConfig.java ---
@@ -26,8 +26,19 @@
public Integer zkPort = null;
public String zkRoot
Github user rick-kilgore commented on the pull request:
https://github.com/apache/storm/pull/254#issuecomment-57557727
@ptgoetz I think I misunderstood your request at first - I merged
everything into master in my fork instead of the "retries" branch, because I
thought that was a requ
Github user rick-kilgore commented on the pull request:
https://github.com/apache/storm/pull/254#issuecomment-57557081
Latest changes from master are merged here now:
https://github.com/apache/storm/pull/276
---
If your project is set up for it, you can reply to this email and ha
GitHub user rick-kilgore reopened a pull request:
https://github.com/apache/storm/pull/254
STORM-495: KafkaSpout retries with exponential backoff
This is an implementation of the improvement request STORM-495:
https://issues.apache.org/jira/browse/STORM-495
The Kafka
GitHub user rick-kilgore opened a pull request:
https://github.com/apache/storm/pull/276
STORM-495: KafkaSpout retries with exponential backoff
This is an implementation of the improvement request STORM-495:
https://issues.apache.org/jira/browse/STORM-495
It was recreate
Github user rick-kilgore commented on the pull request:
https://github.com/apache/storm/pull/254#issuecomment-57536559
I need to create a PR with just the changes that go back to you all. I had
made some changes in my master for deploying to our local maven repo. Expect
to get this
Github user rick-kilgore closed the pull request at:
https://github.com/apache/storm/pull/254
---
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
Github user Parth-Brahmbhatt commented on the pull request:
https://github.com/apache/storm/pull/254#issuecomment-57396672
+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
ena
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/254#issuecomment-57393403
@d2r It looks good to me, I think we just need an up-merge.
@rick-kilgore can you merge your branch with master?
---
If your project is set up for it, you can rep
Github user d2r commented on the pull request:
https://github.com/apache/storm/pull/254#issuecomment-57390466
@Parth-Brahmbhatt , @ptgoetz , @brianeno , @rick-kilgore , I did a quick
pass and it looked OK to me. Where are we in the review of this PR?
---
If your project is set up fo
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/254#discussion_r18250262
--- Diff: external/storm-kafka/src/jvm/storm/kafka/SpoutConfig.java ---
@@ -26,8 +26,19 @@
public Integer zkPort = null;
public String zkRoot = nu
34 matches
Mail list logo
