[GitHub] storm issue #2620: STORM-2994: KafkaSpout doesn't commit offsets for null tu...

2018-04-03 Thread reiabreu
Github user reiabreu commented on the issue: https://github.com/apache/storm/pull/2620 Yeah, going through the Travis logs it does seem they are unstable. Thank you for your help. Much appreciated. ---

[GitHub] storm pull request #2620: STORM-2994: KafkaSpout doesn't commit offsets for ...

2018-04-03 Thread reiabreu
GitHub user reiabreu reopened a pull request: https://github.com/apache/storm/pull/2620 STORM-2994: KafkaSpout doesn't commit offsets for null tuples You can merge this pull request into a Git repository by running: $ git pull https://github.com/reiabreu/storm 1.x-branch

[GitHub] storm pull request #2620: STORM-2994: KafkaSpout doesn't commit offsets for ...

2018-04-03 Thread reiabreu
Github user reiabreu closed the pull request at: https://github.com/apache/storm/pull/2620 ---

[GitHub] storm pull request #2620: STORM-2994: KafkaSpout doesn't commit offsets for ...

2018-04-03 Thread reiabreu
GitHub user reiabreu opened a pull request: https://github.com/apache/storm/pull/2620 STORM-2994: KafkaSpout doesn't commit offsets for null tuples You can merge this pull request into a Git repository by running: $ git pull https://github.com/reiabreu/storm 1.x-branch

[GitHub] storm issue #2593: STORM-2994 KafkaSpout doesn't commit null tuples offsets

2018-04-03 Thread reiabreu
Github user reiabreu commented on the issue: https://github.com/apache/storm/pull/2593 @srdo Sure, I'll have a look ---

[GitHub] storm issue #2593: STORM-2994 KafkaSpout doesn't commit null tuples offsets

2018-04-03 Thread reiabreu
Github user reiabreu commented on the issue: https://github.com/apache/storm/pull/2593 Thank you for guiding me through the changes. I've squashed all the commits and pushed a fresh one ---

[GitHub] storm pull request #2593: STORM-2994 KafkaSpout commit offsets for null tupl...

2018-04-02 Thread reiabreu
Github user reiabreu commented on a diff in the pull request: https://github.com/apache/storm/pull/2593#discussion_r178586564 --- Diff: external/storm-kafka-client/src/test/java/org/apache/storm/kafka/spout/KafkaSpoutNullTupleTest.java --- @@ -0,0 +1,76

[GitHub] storm issue #2593: STORM-2994 KafkaSpout commit offsets for null tuples

2018-04-02 Thread reiabreu
Github user reiabreu commented on the issue: https://github.com/apache/storm/pull/2593 KafkaSpoutAbstractTest is tightly coupled to KafkaSpoutConfig through createSpoutConfig, meaning that to test a single configuration change, we need to create a new test class. This is something

[GitHub] storm pull request #2593: STORM-2994 KafkaSpout commit offsets for null tupl...

2018-04-02 Thread reiabreu
Github user reiabreu commented on a diff in the pull request: https://github.com/apache/storm/pull/2593#discussion_r178559146 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java --- @@ -570,20 +572,25 @@ public void ack(Object

[GitHub] storm pull request #2593: STORM-2994 KafkaSpout commit offsets for null tupl...

2018-04-02 Thread reiabreu
Github user reiabreu commented on a diff in the pull request: https://github.com/apache/storm/pull/2593#discussion_r178510737 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java --- @@ -484,8 +484,11 @@ private boolean emitOrRetryTuple

[GitHub] storm issue #2593: STORM-2994 KafkaSpout commit offsets for null tuples

2018-03-28 Thread reiabreu
Github user reiabreu commented on the issue: https://github.com/apache/storm/pull/2593 Hi folks! I won't have access to a computer until next weekend. I'll pick it up then. Thank you for understanding On Wed, 28 Mar 2018, 08:15 Jungtaek Lim, <notificati...@github.

[GitHub] storm pull request #2593: STORM-2994 KafkaSpout commit offsets for null tupl...

2018-03-17 Thread reiabreu
GitHub user reiabreu opened a pull request: https://github.com/apache/storm/pull/2593 STORM-2994 KafkaSpout commit offsets for null tuples Hello, Let's kick off this pull request. Unit tests for null tuples were missing. I'm in the process of adding them. I'll