[GitHub] storm pull request: STORM-1366. Add documentation for StormSQL int...

2015-12-09 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/931#discussion_r47147496 --- Diff: documentation/storm-sql.md --- @@ -0,0 +1,87 @@ +--- +title: Storm SQL integration +layout: documentation +documentation: true

[GitHub] storm pull request: STORM-1395: move JUnit dependency to top-level...

2015-12-16 Thread ptgoetz
GitHub user ptgoetz opened a pull request: https://github.com/apache/storm/pull/950 STORM-1395: move JUnit dependency to top-level pom https://issues.apache.org/jira/browse/STORM-1395 You can merge this pull request into a Git repository by running: $ git pull https

[GitHub] storm pull request: STORM-1399: Blobstore tests should write data ...

2015-12-17 Thread ptgoetz
GitHub user ptgoetz opened a pull request: https://github.com/apache/storm/pull/955 STORM-1399: Blobstore tests should write data to target so it gets re… …moved when running mvn clean https://issues.apache.org/jira/browse/STORM-1399 You can merge this pull request

[GitHub] storm pull request: STORM-1399: Blobstore tests should write data ...

2015-12-17 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/955#discussion_r47971375 --- Diff: external/storm-hdfs/src/test/java/org/apache/storm/hdfs/blobstore/BlobStoreTest.java --- @@ -76,6 +76,7 @@ @Before public void

[GitHub] storm pull request: [STORM-1418] improve debug logs for some exter...

2016-01-04 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/976#issuecomment-168802302 +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

[GitHub] storm pull request: STORM-1397: Merge conflict from Pacemaker merg...

2016-01-04 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/951#issuecomment-168808016 +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

[GitHub] storm pull request: STORM-1199 : HDFS Spout Functionally complete....

2016-01-04 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/936#discussion_r48782534 --- Diff: pom.xml --- @@ -204,7 +204,7 @@ 0.2.4 3.3.2 0.9.0 -16.0.1 +15.0 --- End diff

[GitHub] storm pull request: STORM-1422 Remove broken example from the tuto...

2016-01-04 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/979#issuecomment-168794540 @d2r I'm fine with updating the instructions. If I remember correctly (it's been a while ;) ), it was to make it very easy for new users to see a topology run in local

[GitHub] storm pull request: [STORM-1413] remove unused variables for some ...

2016-01-04 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/974#issuecomment-168803581 +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

[GitHub] storm pull request: [STORM-1401] removes multilang-test

2016-01-04 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/966#issuecomment-168805521 +1 @d2r Did you create a follow-up JIRA for new multi-lang tests? --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] storm pull request: [STORM-1419] Solr bolt should handle tick tupl...

2016-01-04 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/977#issuecomment-168801983 +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

[GitHub] storm pull request: [STORM-1424] Removed unused topology-path vari...

2016-01-04 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/981#issuecomment-168792176 +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

[GitHub] storm pull request: Update pom.xml

2016-01-04 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/599#issuecomment-168829281 -1 flux-wrappers add convenience multi-lang spout/bolt implementations. Arguably, they could be included in flux-core. I'd rather go that route than set flux-wrappers

[GitHub] storm pull request: STORM-1028: Eventhub spout meta data

2016-01-04 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/651#issuecomment-168831787 @mooso Any update? @rsltrifork Would you be willing to make the change/PR based on the earlier comments? --- If your project is set up for it, you can reply

[GitHub] storm pull request: STORM-706 . Clarify examples README.

2016-01-04 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/465#issuecomment-168825799 +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

[GitHub] storm pull request: [STORM-1210] Set Output Stream id in KafkaSpou...

2016-01-04 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/885#issuecomment-168816433 +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

[GitHub] storm pull request: add cgroup function that can limit cpu share o...

2016-01-04 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/667#issuecomment-168877269 I agree with @d2r, or would at least answer that, yes, we should hold off on cgroups support until the jstorm merge. I believe that @revans2 might have already covered

[GitHub] storm pull request: [STORM-468] java.io.NotSerializableException s...

2016-01-04 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/477#issuecomment-168881795 Ping (mostly to other committers to solicit review/discussion). This is not a bad idea. --- If your project is set up for it, you can reply to this email

[GitHub] storm pull request: STORM-553:add eclipse java formatter file for ...

2016-01-04 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/478#issuecomment-168884966 I'm all for adopting a code style guide for all languages (Java, Clojure, Python, etc.), but I'd rather see a style specification documented first, then follow up

[GitHub] storm pull request: [STORM-1425] Tick tuples must be acked like no...

2016-01-05 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/982#issuecomment-169065098 +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

[GitHub] storm pull request: STORM-1417: fixed equals/hashCode contract in ...

2016-01-05 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/975#issuecomment-169068835 +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

[GitHub] storm pull request: STORM-1348 - refactor API to remove Insert/Upd...

2016-01-05 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/929#issuecomment-169060207 +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

[GitHub] storm pull request: STORM-1406: Add MQTT Support

2016-01-06 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/991#discussion_r48987203 --- Diff: external/storm-mqtt/core/pom.xml --- @@ -0,0 +1,153 @@ + +http://maven.apache.org/POM/4.0.0; xmlns:xsi="http://www.w3.org/2001/XMLS

[GitHub] storm pull request: STORM-1406: Add MQTT Support

2016-01-06 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/991#discussion_r48976202 --- Diff: external/storm-mqtt/core/src/main/java/org/apache/storm/mqtt/common/MqttUtils.java --- @@ -0,0 +1,44 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: STORM-1406: Add MQTT Support

2016-01-06 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/991#discussion_r48976235 --- Diff: external/storm-mqtt/core/src/main/java/org/apache/storm/mqtt/common/MqttOptions.java --- @@ -0,0 +1,334 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: STORM-1406: Add MQTT Support

2016-01-06 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/991#discussion_r48976364 --- Diff: external/storm-mqtt/core/src/main/java/org/apache/storm/mqtt/MqttLogger.java --- @@ -0,0 +1,45 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: STORM-1406: Add MQTT Support

2016-01-06 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/991#discussion_r48976420 --- Diff: external/storm-mqtt/core/src/main/java/org/apache/storm/mqtt/bolt/MqttBolt.java --- @@ -0,0 +1,108 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: STORM-1406: Add MQTT Support

2016-01-07 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/991#discussion_r49160828 --- Diff: external/storm-mqtt/core/src/main/java/org/apache/storm/mqtt/trident/MqttPublishFunction.java --- @@ -0,0 +1,84 @@ +/** + * Licensed

[GitHub] storm pull request: STORM-1406: Add MQTT Support

2016-01-08 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/991#issuecomment-170105203 @satishd, @arunmahadevan, @HeartSaVioR Thanks for your reviews. I believe I've addressed all your comments. I've also added documentation. --- If your project is set up

[GitHub] storm pull request: STORM-1406: Add MQTT Support

2016-01-05 Thread ptgoetz
GitHub user ptgoetz opened a pull request: https://github.com/apache/storm/pull/991 STORM-1406: Add MQTT Support https://issues.apache.org/jira/browse/STORM-1406 You can merge this pull request into a Git repository by running: $ git pull https://github.com/ptgoetz/storm storm

[GitHub] storm pull request: STORM-1406: Add MQTT Support

2016-01-05 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/991#issuecomment-169145828 Initial commit for code review. Further documentation to follow. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] storm pull request: STORM-1199 : HDFS Spout Functionally complete....

2016-01-04 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/936#issuecomment-168812437 Minor nit, but several instances throughout: Can you update the style so that all `if` blocks are enclosed in `{}` (even if it is only one line)? --- If your

[GitHub] storm pull request: [STORM-1412] null check should be done in the ...

2016-01-04 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/970#issuecomment-168813390 +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

[GitHub] storm pull request: [STORM-1427] add TupleUtils/listHashCode metho...

2016-01-04 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/985#issuecomment-168754927 +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

[GitHub] storm pull request: STORM-1406: Add MQTT Support

2016-01-06 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/991#discussion_r49006394 --- Diff: external/storm-mqtt/core/src/main/java/org/apache/storm/mqtt/common/MqttUtils.java --- @@ -0,0 +1,44 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: [STORM-1408] clean up the build directory crea...

2015-12-22 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/964#issuecomment-166693980 +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

[GitHub] storm pull request: Fix blobstore log class

2015-12-22 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/962#issuecomment-16775 +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

[GitHub] storm pull request: STORM-1218: Use markdown for JavaDoc

2015-11-24 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/891#issuecomment-159422903 **Note:** This pull request originally included the following commit: https://github.com/apache/storm/commit/e03b28ca6222f61471b4acb1a6086aab9b597b8a Earlier

[GitHub] storm pull request: STORM-1075 add external module storm-cassandra

2015-11-24 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/827#issuecomment-159353605 @satishd I think that work can be done in follow-up JIRAs after this PR is merged. --- If your project is set up for it, you can reply to this email and have your

[GitHub] storm pull request: STORM-1207: Added flux support for IWindowedBo...

2015-11-24 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/899#issuecomment-159365011 +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

[GitHub] storm pull request: STORM-1075 add external module storm-cassandra

2015-11-24 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/827#issuecomment-159331571 @fhussonnois I think that sounds like a good approach. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] storm pull request: STORM-1218: Use markdown for JavaDoc

2015-11-24 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/891#issuecomment-159371834 I removed the highlight.js license from the source release LICENSE file since generated javadoc is not part of a source release. --- If your project is set up

[GitHub] storm pull request: Update ObjectDef.java for flux

2015-11-24 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/732#issuecomment-159378631 +1 tests can be added later --- 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] storm pull request: STORM-1218: Use markdown for JavaDoc

2015-11-18 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/891#issuecomment-157928854 Re: Merge conflicts, I'm happy to resolve therm at merge time. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] storm pull request: [STORM-885] Heartbeat Server (Pacemaker)

2015-11-18 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/838#issuecomment-157914621 @knusbaum Nice documentation! Thank you. Some thoughts on documentation improvements (please correct me if I'm wrong): 1. Since pacemaker is a single

[GitHub] storm pull request: STORM-1218: Use markdown for JavaDoc

2015-11-18 Thread ptgoetz
GitHub user ptgoetz opened a pull request: https://github.com/apache/storm/pull/891 STORM-1218: Use markdown for JavaDoc JIRA: https://issues.apache.org/jira/browse/STORM-1218 This is a first pass at converting the JavaDoc in `storm-core` to markdown, using [pegdown-doclet

[GitHub] storm pull request: STORM-1213: Remove sigar binaries from source ...

2015-11-18 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/887#issuecomment-157880047 @revans2 Ah... okay. So we need to change `resources/resources` to just `resources`, and remove the `-1.6.4` from the libraries, correct? That should be doable

[GitHub] storm pull request: STORM-1213: Remove sigar binaries from source ...

2015-11-19 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/887#issuecomment-158184985 @revans2 I couldn't get the maven dependency plugin to do what I wanted, so I changed tactics. I switched to using the maven antrun plugin to download and unpack

[GitHub] storm pull request: [STORM-885] Heartbeat Server (Pacemaker)

2015-11-19 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/838#issuecomment-158193770 @knusbaum Nice work on the documentation. Thank you. +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] storm pull request: STORM-1213: Remove sigar binaries from source ...

2015-11-18 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/887#issuecomment-157868784 @revans2 How are you running it (i.e. local/remote/unit test)? My first thought is that we may just need to bind the dependency plugin to a different maven lifecycle

[GitHub] storm pull request: STORM-1213: Remove sigar binaries from source ...

2015-11-18 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/887#issuecomment-157869384 @revans2 That directory structure is what's in master right now: https://github.com/apache/storm/tree/master/external/storm-metrics/src/main/resources/resources

[GitHub] storm pull request: STORM-1218: Use markdown for JavaDoc

2015-11-19 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/891#issuecomment-158109736 @revans2 Yes, it is inserted into the generated HTML. Since javadoc isn't part of the source release, it probably only needs to be added to the LICENSE for the binary

[GitHub] storm pull request: [STORM-1175] State store for windowing operati...

2016-01-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/939#issuecomment-171028002 +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

[GitHub] storm pull request: [STORM-1176] Checkpoint window evaluated/expir...

2016-01-12 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/963#discussion_r49506460 --- Diff: examples/storm-starter/src/jvm/storm/starter/StatefulWindowingTopology.java --- @@ -0,0 +1,110 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: STORM-587. trident transactional state in zk s...

2016-01-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/475#issuecomment-171048834 Unless others feel strongly about this change, I think it's okay to close this. --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] storm pull request: [STORM-1176] Checkpoint window evaluated/expir...

2016-01-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/963#issuecomment-171044220 +1 aside from 2 very minor nits, but the feature is sizable so I'd like to see additional committer reviews. I'd also like to see this included in the 1.0

[GitHub] storm pull request: [STORM-1176] Checkpoint window evaluated/expir...

2016-01-12 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/963#discussion_r49507663 --- Diff: storm-core/src/jvm/org/apache/storm/state/StateProvider.java --- @@ -0,0 +1,38 @@ +/** + * Licensed to the Apache Software Foundation (ASF

[GitHub] storm pull request: STORM-919 Gathering worker and supervisor proc...

2016-01-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/608#issuecomment-171049571 @bourneagain Any update on 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

[GitHub] storm pull request: [STORM-1453] nimbus.clj/wait-for-desired-code-...

2016-01-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/1003#issuecomment-171045749 @revans2 looks like this was originally #996, which had the requisite +1s, but needed an upmerge. I think @caofangkun just opened a new pull request and closed #996

[GitHub] storm pull request: Merge changes from {master}/docs to {asf-site}

2016-01-12 Thread ptgoetz
GitHub user ptgoetz opened a pull request: https://github.com/apache/storm/pull/1008 Merge changes from {master}/docs to {asf-site} JIRA: https://issues.apache.org/jira/browse/STORM-1468 You can merge this pull request into a Git repository by running: $ git pull https

[GitHub] storm pull request: STORM-1468: remove {master}/docs

2016-01-12 Thread ptgoetz
GitHub user ptgoetz opened a pull request: https://github.com/apache/storm/pull/1009 STORM-1468: remove {master}/docs JIRA: https://issues.apache.org/jira/browse/STORM-1468 The content of the {master}/docs directory is now in the {asa-site} branch. You can merge this pull

[GitHub] storm pull request: STORM-1468: remove {master}/docs

2016-01-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/1009#issuecomment-171126186 Assuming lazy consensus. --- 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] storm pull request: STORM-1468: remove {master}/docs

2016-01-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/1009#issuecomment-171126632 See also #1008 --- 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] storm pull request: STORM-1468: Merge changes from {master}/docs t...

2016-01-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/1008#issuecomment-171126410 Assuming lazy consensus, but would appreciate review in case I missed anything. --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] storm pull request: STORM-1468: Merge changes from {master}/docs t...

2016-01-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/1008#issuecomment-171126503 See also #1009 --- 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] storm issue #1456: [STORM-1874]Update logger private permissions

2016-06-08 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1456 +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

[GitHub] storm issue #1473: STORM-1864 : StormSubmitter should throw respective excep...

2016-06-08 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1473 +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

[GitHub] storm issue #1324: STORM-1700 Introduce 'whitelist' / 'blacklist' option to ...

2016-06-08 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1324 See my comments on #1325 as they apply here 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

[GitHub] storm pull request #1325: STORM-1700 Introduce 'whitelist' / 'blacklist' opt...

2016-06-08 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/1325#discussion_r66292563 --- Diff: conf/storm.yaml.example --- @@ -39,10 +39,13 @@ # - "server2" ## Metrics Consumers +## NOTE: task queue will be

[GitHub] storm issue #1471: STORM-1865: update command line client document

2016-06-08 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1471 +1 nice job! --- 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

[GitHub] storm pull request #1325: STORM-1700 Introduce 'whitelist' / 'blacklist' opt...

2016-06-08 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/1325#discussion_r66292249 --- Diff: storm-core/src/clj/org/apache/storm/daemon/common.clj --- @@ -298,18 +299,21 @@ {[comp-id METRICS-STREAM-ID] :shuffle

[GitHub] storm issue #1453: STORM-1873 Implement alternative behaviour for late tuple...

2016-06-08 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1453 +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

[GitHub] storm issue #1379: STORM-1742 (1.x) More accurate 'complete latency'

2016-06-08 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1379 I wouldn't mind seeing some larger-scale performance tests, since it doubles the size of ACK tuples (I know, they're still small). If others can confirm the absence of any performance degradation

[GitHub] storm pull request #1331: STORM-1705: Cap number of retries for a failed mes...

2016-06-08 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/1331#discussion_r66318933 --- Diff: external/storm-kafka/src/jvm/org/apache/storm/kafka/ExponentialBackoffMsgRetryManager.java --- @@ -86,15 +94,23 @@ public Long

[GitHub] storm pull request #1331: STORM-1705: Cap number of retries for a failed mes...

2016-06-08 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/1331#discussion_r66318510 --- Diff: external/storm-kafka/src/jvm/org/apache/storm/kafka/SpoutConfig.java --- @@ -37,6 +37,8 @@ public long retryInitialDelayMs = 0

[GitHub] storm issue #1331: STORM-1705: Cap number of retries for a failed message

2016-06-08 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1331 Two minor nits. I'm +1 once they are addressed. --- 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] storm issue #1468: STORM-1885. python script for squashing and merging prs.

2016-06-06 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1468 @harshach The source of the file is referenced here: https://github.com/apache/storm/pull/1468/files#diff-da45fe3972445a9f82ef768808dd8853R20 I'd like to get clearance that what

[GitHub] storm issue #1464: STORM-1884: Prioritize pendingPrepare over pendingCommit

2016-06-07 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1464 +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

[GitHub] storm issue #1461: [STORM-1882] Expose TextFileReader public

2016-06-07 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1461 +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

[GitHub] storm issue #1468: STORM-1885. python script for squashing and merging prs.

2016-06-06 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1468 > It will ask for primary authors and the user who is merging this can input more than one author at the time of merge. That means it removes authorship information. If we tag a squas

[GitHub] storm issue #1458: STORM-1878: Flux can now handle IStatefulBolts

2016-06-06 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1458 +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

[GitHub] storm issue #1468: STORM-1885. python script for squashing and merging prs.

2016-06-06 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1468 I'm a little on the fence in terms of squashing the commits of others vs. asking the contributor to do so. There are a lot of situations where spreading out a big patch over multiple commits makes

[GitHub] storm issue #1481: STORM-1705: Fix cap-retry bug

2016-06-10 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1481 +1 Thanks @abhishekagarwal87. Since this is a one-line change blocking the release, I'm going to merge it in the interest of proceeding with the release. If there are any objections we can

[GitHub] storm issue #1325: STORM-1700 Introduce 'whitelist' / 'blacklist' option to ...

2016-06-10 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1325 +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

[GitHub] storm issue #1324: STORM-1700 Introduce 'whitelist' / 'blacklist' option to ...

2016-06-10 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1324 +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

[GitHub] storm issue #1467: STORM-1771. HiveState should flushAndClose before closing...

2016-06-09 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1467 +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

[GitHub] storm issue #1467: STORM-1771. HiveState should flushAndClose before closing...

2016-06-09 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1467 There was a compilation issue with this patch. I fixed it with this commit: 0b54767 Please review. If there are any objections I can revert. --- If your project is set up for it, you can reply

[GitHub] storm issue #1331: STORM-1705: Cap number of retries for a failed message

2016-06-09 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1331 +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

[GitHub] storm pull request: [STORM-1766] - A better algorithm server rack ...

2016-05-25 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/1398#issuecomment-221677136 @jerrypeng Did you merge this to any other branches, or just master? --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] storm pull request: STORM-1861 Fix storm script bug to not fork ja...

2016-05-25 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/1441#discussion_r64637467 --- Diff: bin/storm.py --- @@ -241,7 +240,6 @@ def jar(jarfile, klass, *args): extrajars=[tmpjar, USER_CONF_DIR, STORM_BIN_DIR

[GitHub] storm pull request: [STORM-1766] - A better algorithm server rack ...

2016-05-25 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/1398#issuecomment-221686373 @jerrypeng For tracking what goes into each branch/release. Github only gives us merge notifications for the branch a pull request targeted. If you had merged

[GitHub] storm issue #1468: STORM-1885. python script for squashing and merging prs.

2016-06-06 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1468 > We won't be able capture this in JIRA either. I am not sure how much of this is important to have all the commits from each contributor for a single JIRA which in itself is rare unless its a

[GitHub] storm issue #1153: [STORM-1575] fix TwitterSampleSpout NPE on close

2016-06-07 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1153 +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

[GitHub] storm pull request: STORM-1864 StormSubmitter should throw respect...

2016-05-25 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/1446#issuecomment-221764594 +1 Thanks for the clarification @satishd! --- 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] storm pull request: STORM-1861 Fix storm script bug to not fork ja...

2016-05-27 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/1448#issuecomment-222169053 +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

[GitHub] storm pull request: [STORM-1868] Modify TridentKafkaWordCount to r...

2016-05-27 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/1449#issuecomment-01561 +1 This should probably also be applied to the 1.* branches. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] storm pull request: STORM-1466: Move the org.apache.thrift7 namesp...

2016-01-13 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/1007#issuecomment-171365717 +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

[GitHub] storm pull request: STORM-1467: Switch apache-rat plugin off by de...

2016-01-13 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/1006#issuecomment-171366113 +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

[GitHub] storm pull request: [STORM-1452] Fixes profiling/debugging out of ...

2016-01-14 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/1012#issuecomment-171816513 Works for me. Thanks @d2r. +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

[GitHub] storm pull request: [STORM-1478] make bolt's getComponentConfigura...

2016-01-18 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/1020#issuecomment-172728583 +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

<    1   2   3   4   5   6   7   >