[GitHub] storm issue #2428: STORM-2826: Set key/value deserializer fields when using ...

2017-12-08 Thread hmcl
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2428 @srdo aiming to getting this PR merged more quickly I created a [PR](https://github.com/srdo/storm/pull/1) with a suggested fix off your branch. If you agree with the fix, can you please incorporate it,

[GitHub] storm pull request #2428: STORM-2826: Set key/value deserializer fields when...

2017-12-08 Thread hmcl
Github user hmcl commented on a diff in the pull request: https://github.com/apache/storm/pull/2428#discussion_r155909342 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutConfig.java --- @@ -359,7 +356,7 @@ private Builder(Builder builder,

[GitHub] storm issue #2203: STORM-2153: New Metrics Reporting API

2017-12-08 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2203 @ptgoetz I think that is a good start. Not sure if users would be upset that their bolt B.1 now looks like B_1. We might have issues though if someone has both a B.1 bolt and a B_1 bolt. It might

[GitHub] storm issue #2203: STORM-2153: New Metrics Reporting API

2017-12-08 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/2203 @revans2 Thanks for clarifying. I have a better understanding of what you're saying now. How do you feel about just replacing "." with "_" on all metrics path components (host name, componen

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-12-08 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r155864521 --- Diff: storm-core/src/jvm/org/apache/storm/metrics2/reporters/ScheduledStormReporter.java --- @@ -0,0 +1,90 @@ +/** + * Licensed to the Apache So

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-12-08 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r155863202 --- Diff: storm-core/src/jvm/org/apache/storm/validation/ConfigValidation.java --- @@ -493,6 +493,46 @@ public void validateField(String name, Object o) {

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-12-08 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r155863192 --- Diff: storm-core/src/jvm/org/apache/storm/validation/ConfigValidation.java --- @@ -493,6 +493,46 @@ public void validateField(String name, Object o) {

[GitHub] storm issue #2203: STORM-2153: New Metrics Reporting API

2017-12-08 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2203 @ptgoetz for me this is all about the metadata for the metric. The code we have been working on for storing metrics in a DB that we can then use for scheduling is getting close to being done.

[GitHub] storm issue #2451: STORM-2850: Make ManualPartitionSubscription call rebalan...

2017-12-08 Thread hmcl
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2451 +1 ---

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-12-08 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r155856433 --- Diff: storm-core/src/jvm/org/apache/storm/validation/ConfigValidation.java --- @@ -493,6 +493,46 @@ public void validateField(String name, Object o) {

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-12-08 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r155855673 --- Diff: storm-core/src/jvm/org/apache/storm/metrics2/reporters/ScheduledStormReporter.java --- @@ -0,0 +1,90 @@ +/** + * Licensed to the Apache So

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-12-08 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r155855300 --- Diff: storm-core/src/jvm/org/apache/storm/metrics2/reporters/ScheduledStormReporter.java --- @@ -0,0 +1,90 @@ +/** + * Licensed to the Apache So

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-12-08 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r155854567 --- Diff: storm-core/src/jvm/org/apache/storm/metrics2/reporters/GangliaStormReporter.java --- @@ -0,0 +1,136 @@ +/** + * Licensed to the Apache Sof

[GitHub] storm issue #2203: STORM-2153: New Metrics Reporting API

2017-12-08 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/2203 @revans2 (cc @HeartSaVioR ) Regarding metadata, parseable metrics names/paths, etc. what do you think of the following approach? In a nutshell, make everything configurable (with sane default

[GitHub] storm pull request #2451: STORM-2850: Make ManualPartitionSubscription call ...

2017-12-08 Thread srdo
GitHub user srdo opened a pull request: https://github.com/apache/storm/pull/2451 STORM-2850: Make ManualPartitionSubscription call rebalance listener… … on revoke hook before assigning new partitions to the consumer See https://issues.apache.org/jira/browse/STORM-2850.

[GitHub] storm issue #2409: STORM-2796: Flux: Provide means for invoking static facto...

2017-12-08 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/2409 @hmcl I'm on the same page regarding needs for squashing: even in this PR I squashed the commits before merging. I also support merge script and I have been considering to initiate discussio

[GitHub] storm issue #2409: STORM-2796: Flux: Provide means for invoking static facto...

2017-12-08 Thread hmcl
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/2409 @HeartSaVioR there is not one major project that does not require contributors to merge commits. It takes a few minutes and it makes a world of difference in terms of making the git log easy to understa