Github user hmcl commented on the pull request:
https://github.com/apache/storm/pull/1448#issuecomment-222035081
+1. @csivaguru can you please squash the commits. It will keep the log a
bit cleaner
---
If your project is set up for it, you can reply to this email and have your
reply
Github user hmcl commented on the pull request:
https://github.com/apache/storm/pull/1131#issuecomment-78070
@jianbzhou I confirm that your suggested fix for
doSeekRetriableTopicPartitions is correct. I am going to include that in the
next patch.
---
If your project is set up
Github user hmcl commented on the pull request:
https://github.com/apache/storm/pull/1131#issuecomment-78660
@connieyang I am finishing addressing some issues brought up by the initial
users of this kafka spout, as well as unit test coverage, and will push the
trident API right
Github user hmcl commented on the pull request:
https://github.com/apache/storm/pull/1131#issuecomment-80056
@jianbzhou any updates on
``` One customer of us who also use the spout they found some other issues:
1. Work load is not distributed to all spout tasks(as per
Github user hmcl commented on the pull request:
https://github.com/apache/storm/pull/1131
@jianbzhou **Storm** guarantees that all the messages are either acked or
failed. There is the property "topology.message.timeout.secs"
https://github.com/apache/storm/blob/master/stor
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/1352
+1. I am also in favor of making cluster summary modular.
---
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 hmcl commented on the issue:
https://github.com/apache/storm/pull/1419
@flisky some stuff came up. Apologies for the delay in my response. I will
get back to you today or tomorrow as I am working on this now.
---
If your project is set up for it, you can reply to this
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/1131
@jianbzhou thanks. Looking at 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
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/1451#discussion_r65576093
--- Diff:
external/storm-kafka-monitor/src/main/java/org/apache/storm/kafka/monitor/KafkaOffsetLagUtil.java
---
@@ -0,0 +1,374 @@
+/*
+ * Licensed to
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/1451#discussion_r65576345
--- Diff:
external/storm-kafka-monitor/src/main/java/org/apache/storm/kafka/monitor/KafkaOffsetLagUtil.java
---
@@ -0,0 +1,374 @@
+/*
+ * Licensed to
GitHub user hmcl opened a pull request:
https://github.com/apache/storm/pull/1491
STORM-1907: PartitionedTridentSpoutExecutor has incompatible types that
cause ClassCastException
- Generify on Object rather than Integer because example is broken
- Fix TridentWordCount example
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/1491
This PR is related to https://github.com/apache/storm/pull/1429 but I found
this issue independently, in my local branch, which was a bit outdated. This
patch still makes sense since it gets rid of
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/1488
+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
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/1431
+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
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/1487
@nabhanelrahman I am addressing this and other edge cases in a patch that I
am about to submit. I am afraid that just removing the +1 won't work for all
the cases.
---
If your project is set u
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/1474
@harshach @nabhanelrahman looking at this now. Somehow I missed this PR
earlier when going throw the PRs for reviewing.
---
If your project is set up for it, you can reply to this email and have your
GitHub user hmcl opened a pull request:
https://github.com/apache/storm/pull/1516
STORM-1930: Kafka New Client API - Support for Topic Wildcards
- Interfaces for TuplesBuilder and KafkaSpoutStreams and implementations
for named topics and wildcards
- Test topologies for named
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/1516
@harshach Should this patch be merged onto 1.x-branch as well. 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
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/1474
@nabhanelrahman your patch enlightened that it would be better to add the
`emit` and `getOutputFields` methods to `KafkaSpoutStream`. Furthermore, there
were a couple of design decisions that would be
GitHub user hmcl opened a pull request:
https://github.com/apache/storm/pull/1556
STORM-1737: storm-kafka-client has compilation errors with Apache Kafka 0.10
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/hmcl/storm-apache
GitHub user hmcl opened a pull request:
https://github.com/apache/storm/pull/1562
TridentHiveTopology fails to run due to incompatibility between thrift
dependencies
- hive-hcatalog-streaming:jar depends on thrift 0.9.3
- storm-hive:jar depends on thrift 0.9.0
- Update
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/1562
This fix likely has to be backported to 1.x-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
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/1482
+1 besides the neat pick which would be to have the new spout property
names prefixed by `storm.kafka.client` property names for consistency.
---
If your project is set up for it, you can reply to
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/1482#discussion_r70818175
--- Diff: pom.xml ---
@@ -258,6 +258,12 @@
1.4.0-incubating
2.6.3
2.18.1
+
+
+0.8.2.1
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/1556
@harshach @HeartSaVioR @abhishekagarwal87, thank you for your feedback. The
updated ReadME, including the added Wildcard Support will be out in a few mins.
---
If your project is set up for it, you
GitHub user hmcl opened a pull request:
https://github.com/apache/storm/pull/1576
Kafka Spout New Consumer API - KafkaSpoutRetryExponentialBackoff Bug
- KafkaSpoutRetryExponentialBackoff#nextTime(...) method is off by order
of magnitude
You can merge this pull request into a Git
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/1576
Please merge this into 1.x-branch as well
---
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
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/1576
@harshach @priyank5485 can you please review. 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
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/1576
@priyank5485 thanks for catch.
---
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
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/1587
@darionyaphet can you please clarify why this patch is needed? In my
understanding it shouldn't be necessary to have any logic for auto commit
interval. If one wishes to set the auto commit int
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/1587
@darionyaphet this kafka spout does not use zookeeper as the new Kafka API
writes the offsets to a topic rather than to zookeeper. As I mentioned in the
initial comment, this change is not necessary
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/1131
@jianbzhou you can get obtain at most once semantics by setting maxRetries
to zero. Here is the method to do so.
https://github.com/apache/storm/blob/master/external/storm-kafka-client/src
GitHub user hmcl opened a pull request:
https://github.com/apache/storm/pull/1649
STORM-2052: Kafka Spout - New Client API - Performance Improvements
This patch should be back-ported to 1.0.x branch and 1.x-branch. Thank you.
You can merge this pull request into a Git repository by
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/1649#discussion_r76275357
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java
---
@@ -221,7 +224,22 @@ private boolean commit
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/1649
@harshach I can squash the commits, but I left these three commits on
purpose. Each commit is a logical groping. One is docs, one is the default
values, and one is the logging changes. They all have
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/1649
@harshach Done!
---
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
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2271
@srdo shouldn't this [PR](https://github.com/apache/storm/pull/2174) come
in before this one?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitH
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2174
@srdo thanks for your comments. Let me think about it. This change builds
on the existing code and fixes some of the problems. I was also planning on
revisiting this implementation. The way to go about
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2276
+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
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2174
@srdo I am evaluating if we can do the change without breaking the API. If
so we can go ahead with it. Otherwise, as you suggested, we can go with this
change for 1.x-branch and then refactor for Storm
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2275
+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
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2277
@srdo can you please update the JIRA ticket, 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
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2181
@tiodollar once you do the merge, can you please squash all the commits.
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
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2273
@momoxixi what's the motivation behind merging 1.1.x-branch onto master?
---
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 pr
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2282#discussion_r135032704
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/EmptyKafkaTupleListener.java
---
@@ -0,0 +1,54 @@
+/*
+ * Licensed to
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2282#discussion_r135033155
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/EmptyKafkaTupleListener.java
---
@@ -0,0 +1,33 @@
+package
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2174
@srdo I think that you should have spoken with me before implementing this.
This task is assigned to me, and I have an implementation that is pending
addressing a few changes. There was no timeframe
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2300
@srdo this change should go on https://github.com/apache/storm/pull/2174
---
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
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2249
@srdo apologies for the delay. I will finish today.
---
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2367#discussion_r144044782
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/internal/OffsetManager.java
---
@@ -53,8 +54,8 @@
public
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2367#discussion_r144066568
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/internal/OffsetManager.java
---
@@ -53,8 +54,8 @@
public
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2367
@srdo I understand that you want to keep the commit history for @tiodollar
but I don't think we should have 4 commits for such a simple change. This patch
should be one commit only, either you
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2367
@srdo in my opinion the most important thing is to keep the project, and in
particular the git log clean. I often have to cherry-pick changes from Apache
branch into my own branch, and multiple commits
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2367#discussion_r144078200
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/internal/OffsetManager.java
---
@@ -53,8 +54,8 @@
public
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2367#discussion_r144079454
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/internal/OffsetManager.java
---
@@ -163,8 +167,9 @@ public long commit
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2367
@srdo are you on Gitter or some sort of chat room where I could ask you a
few questions? Thanks.
---
GitHub user hmcl opened a pull request:
https://github.com/apache/storm/pull/2380
STORM-2781: Refactor storm-kafka-client KafkaSpout Processing Guarantees
- Define processing guarantees as AT_LEAST_ONCE, AT_MOST_ONCE, NONE
- Refactor method name from
GitHub user hmcl opened a pull request:
https://github.com/apache/storm/pull/2381
STORM-2784: storm-kafka-client KafkaTupleListener method onPartitionsâ¦
â¦Reassigned() should be called after initialization is complete
You can merge this pull request into a Git repository by
GitHub user hmcl opened a pull request:
https://github.com/apache/storm/pull/2387
STORM-2787: storm-kafka-client KafkaSpout method onPartitionsRevoked(...)
should set initialized flag independently of processing guarantees
You can merge this pull request into a Git repository by
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2385
@govind-menon why is YSTORM-2725 prefixed by Y? Should it be STORM-2725? If
so, can you please fix the typo. Thanks.
---
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2380#discussion_r146996683
--- Diff: docs/storm-kafka-client.md ---
@@ -298,25 +298,44 @@ Currently the Kafka spout has has the following
default values, which have been
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2380#discussion_r146996955
--- Diff: docs/storm-kafka-client.md ---
@@ -298,25 +298,44 @@ Currently the Kafka spout has has the following
default values, which have been
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2380#discussion_r146997051
--- Diff: docs/storm-kafka-client.md ---
@@ -298,25 +298,44 @@ Currently the Kafka spout has has the following
default values, which have been
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2380#discussion_r146997432
--- Diff: docs/storm-kafka-client.md ---
@@ -298,25 +298,44 @@ Currently the Kafka spout has has the following
default values, which have been
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2380#discussion_r147012458
--- Diff: docs/storm-kafka-client.md ---
@@ -298,25 +298,44 @@ Currently the Kafka spout has has the following
default values, which have been
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2380#discussion_r147012467
--- Diff: docs/storm-kafka-client.md ---
@@ -298,25 +298,44 @@ Currently the Kafka spout has has the following
default values, which have been
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2380#discussion_r147012486
--- Diff: docs/storm-kafka-client.md ---
@@ -298,25 +298,44 @@ Currently the Kafka spout has has the following
default values, which have been
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2380#discussion_r147012506
--- Diff: docs/storm-kafka-client.md ---
@@ -298,25 +298,44 @@ Currently the Kafka spout has has the following
default values, which have been
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2380#discussion_r147012706
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java
---
@@ -78,17 +78,17 @@
private transient
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2380#discussion_r147013419
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java
---
@@ -78,17 +78,17 @@
private transient
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2380#discussion_r147013385
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java
---
@@ -78,17 +78,17 @@
private transient
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2380#discussion_r147013620
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java
---
@@ -125,8 +125,8 @@ public void open(Map conf
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2380#discussion_r147013717
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java
---
@@ -255,26 +255,25 @@ private void
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2380#discussion_r147014032
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java
---
@@ -336,22 +335,25 @@ private void emit
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2380#discussion_r147015173
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java
---
@@ -438,55 +440,53 @@ private void
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2380#discussion_r147015407
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java
---
@@ -438,55 +440,53 @@ private void
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2380#discussion_r147015438
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java
---
@@ -438,55 +440,53 @@ private void
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2380#discussion_r147016997
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java
---
@@ -438,55 +440,53 @@ private void
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2380#discussion_r147022902
--- Diff: docs/storm-kafka-client.md ---
@@ -298,25 +298,44 @@ Currently the Kafka spout has has the following
default values, which have been
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2380
I have squashed the commits and addressed the early return issue.
---
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2387
@srdo @HeartSaVioR I have incorporated the code review changes of the
depending patch. It should be good to merge. Thanks.
---
GitHub user hmcl opened a pull request:
https://github.com/apache/storm/pull/2393
STORM-2781: Refactor storm-kafka-client KafkaSpout Processing Guarantees
- Define processing guarantees as AT_LEAST_ONCE, AT_MOST_ONCE, NONE
- Refactor method name from
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2393
@HeartSaVioR for 1.x-branch
---
GitHub user hmcl opened a pull request:
https://github.com/apache/storm/pull/2394
1.x branch storm 2787 ks init flag
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/hmcl/storm-apache
1.x-branch_STORM-2787_KSInitFlag
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2394
@HeartSaVioR for 1.x-branch
---
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2427
+1
---
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2428#discussion_r151886343
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutConfig.java
---
@@ -292,17 +292,21 @@ private Builder(String
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2426#discussion_r151887752
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutConfig.java
---
@@ -718,7 +718,13 @@ private static void
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2426
+1. Thanks @srdo. Can you please squash the commits before merging. Thanks.
---
GitHub user hmcl opened a pull request:
https://github.com/apache/storm/pull/2438
STORM-2835: storm-kafka-client KafkaSpout can fail to remove all tuples
from waitingToEmit
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/hmcl
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2438
Yes, you are right. Technically it is not necessary to remove the elements
from the collection. Nevertheless, this code change addresses the following,
important, issues:
1. Calling next
GitHub user hmcl opened a pull request:
https://github.com/apache/storm/pull/2440
STORM-2784: storm-kafka-client KafkaTupleListener method
onPartitionsReassigned() should be called after initialization is complete
Somehow this change got reverted during a merge in 1.x-branch. It is
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2428
@HeartSaVioR @srdo I will finalize my review by Monday PST
---
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2409
@ptgoetz I would suggest putting in different patches the that the minor
refactoring around coding style and adding adding Javadoc. Also, can you please
squash the commits once you have the necessary
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2438
@srdo @HeartSaVioR thanks for the review. Can you please take another look.
Thanks.
---
Github user hmcl commented on a diff in the pull request:
https://github.com/apache/storm/pull/2438#discussion_r155007745
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java
---
@@ -242,9 +242,7 @@ public void nextTuple
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2448
@Ethanlm can you please "Quick Fix" with "MINOR: " Thanks.
+1 after fixing the commit message.
---
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2448
@Ethanlm yeah, I noticed it right after my comment. Somehow I had not
refreshed my PRs view. If it is easy I will change the commit message since I
am about to merge something. Otherwise we will just
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2428
@arunmahadevan I am looking into this now. Thanks.
---
Github user hmcl commented on the issue:
https://github.com/apache/storm/pull/2438
@arunmahadevan @HeartSaVioR @srdo this patch has been merged into master
and 1.x-branch
---
501 - 600 of 813 matches
Mail list logo
