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

2016-01-20 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/1012#issuecomment-173361855 @d2r You can just delete the .md file in question. I should have a pull request for the docs in a minute. --- If your project is set up for it, you can reply

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

2016-01-20 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/1012#issuecomment-173363553 PR for moving the docs to `asf-site`: https://github.com/apache/storm/pull/1032 That includes the typo @revans2 mentioned. --- If your project is set up

[GitHub] storm pull request: [STORM-1450] - Fix bugs and refactor code in R...

2016-01-20 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/1016#issuecomment-173382241 +1 for backporting 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

[GitHub] storm pull request: STORM-1452: update REST API docs for profiling

2016-01-20 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/1032#issuecomment-173380332 As this is a documentation-only patch, and was previously reviewed as part of #1012, I'm going to merge this. If there are any objections, we can revert the change

[GitHub] storm pull request: [STORM-1450] - Fix bugs and refactor code in R...

2016-01-19 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/1016#issuecomment-172988140 @jerrypeng Can you add this to the CHANGELOG? --- 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-1214: add javadoc for Trident Streams an...

2016-01-19 Thread ptgoetz
GitHub user ptgoetz opened a pull request: https://github.com/apache/storm/pull/1029 STORM-1214: add javadoc for Trident Streams and Operations JIRA: https://issues.apache.org/jira/browse/STORM-1214 This is just a work in progress for STORM-1214, if this is merged, please

[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

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

2016-01-18 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/991#issuecomment-172727308 @arunmahadevan Thanks. Yes, the revans2 commit is due to upmerge. I'm happy to squash some commits, but some are there to facilitate merging to multiple branches

[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-1470] Applies shading to hadoop-auth, c...

2016-01-14 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/1010#issuecomment-171780642 +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-171793561 Not really specific to this pull request, but I missed it before when the profiling functionality was originally added. If profiling is enabled, we turn

[GitHub] storm pull request: [STORM-1470] Applies shading to hadoop-auth, c...

2016-01-14 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/1011#issuecomment-171796975 Personally, I don't think we need separate pull requests for each branch yet (master and 1.x have not diverged enough to cause conflicts), as it kind of confuses things

[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-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 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-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-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-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-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-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-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-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-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-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-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-1373] Blobstore API sample example usag...

2015-12-09 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/934#discussion_r47145053 --- Diff: examples/storm-starter/src/jvm/storm/starter/BlobStoreAPIWordCountTopology.java --- @@ -0,0 +1,248 @@ +/** + * Licensed to the Apache

[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_r47147232 --- Diff: documentation/storm-sql.md --- @@ -0,0 +1,87 @@ +--- +title: Storm SQL integration +layout: documentation +documentation: true

[GitHub] storm pull request: [STORM-1373] Blobstore API sample example usag...

2015-12-09 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/934#discussion_r47144348 --- Diff: examples/storm-starter/src/jvm/storm/starter/BlobStoreAPIWordCountTopology.java --- @@ -0,0 +1,248 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: [STORM-1373] Blobstore API sample example usag...

2015-12-09 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/934#discussion_r47144504 --- Diff: examples/storm-starter/src/jvm/storm/starter/BlobStoreAPIWordCountTopology.java --- @@ -0,0 +1,248 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: Storm 1379 Removed Redundant Structure

2015-12-09 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/932#issuecomment-163380100 +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-1373] Blobstore API sample example usag...

2015-12-09 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/934#discussion_r47144860 --- Diff: examples/storm-starter/src/jvm/storm/starter/BlobStoreAPIWordCountTopology.java --- @@ -0,0 +1,248 @@ +/** + * Licensed to the Apache

[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_r47147827 --- Diff: documentation/storm-sql-internal.md --- @@ -0,0 +1,55 @@ +--- +title: The Internals of Storm SQL +layout: documentation

[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_r47146441 --- Diff: documentation/storm-sql.md --- @@ -0,0 +1,87 @@ +--- +title: Storm SQL integration +layout: documentation +documentation: true

[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-756 Handle taskids response as soon as p...

2015-12-03 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/897#issuecomment-161795108 @d2r I was able to build master before the revert without any problem. Or is what you are seeing only manifest in the TravisCI environment? --- If your project is set

[GitHub] storm pull request: [STORM-876] Blobstore API

2015-12-03 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/845#issuecomment-161736488 @redsanket First of all thank you for contributing this and making it through the most epic code review we've had to date. :) I'm +1 for merging

[GitHub] storm pull request: STORM-1353 Update "jstorm-import" branch to th...

2015-12-02 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/904#issuecomment-161361062 @wuchong Good to hear. Visually they look nearly identical, and the license looks compatible. --- If your project is set up for it, you can reply to this email and have

[GitHub] storm pull request: STORM-1353 Update "jstorm-import" branch to th...

2015-12-01 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/904#issuecomment-16124 Thanks @bastiliu I merged this into the jstorm-import. **One important thing to note:** This pull request introduced highchart.js which has a commercial

[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-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-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-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-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-904: Move bin/storm command line to java...

2015-11-17 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/662#issuecomment-157395059 Any update on this PR? It would seem it at least needs an upmerge and additional review. --- If your project is set up for it, you can reply to this email and have your

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

2015-11-17 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/651#issuecomment-157396019 @tandrup Any update on this? Any thoughts regarding @mooso's comment? --- If your project is set up for it, you can reply to this email and have your reply appear

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

2015-11-17 Thread ptgoetz
GitHub user ptgoetz opened a pull request: https://github.com/apache/storm/pull/887 STORM-1213: Remove sigar binaries from source tree See https://issues.apache.org/jira/browse/STORM-1213 for description. Also removes the LICENSE entry for sigar, since I believe

[GitHub] storm pull request: STORM-1167: Add windowing support for storm co...

2015-11-13 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/855#discussion_r44814116 --- Diff: storm-core/src/jvm/backtype/storm/topology/TopologyBuilder.java --- @@ -177,6 +179,21 @@ public BoltDeclarer setBolt(String id, IBasicBolt bolt

[GitHub] storm pull request: STORM-1203. worker metadata file creation does...

2015-11-13 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/878#issuecomment-156498567 +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-885] Heartbeat Server (Pacemaker)

2015-11-13 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/838#issuecomment-156498367 I'm +1 for adding this functionality, but I would say that vote is non-binding since this is new code/functionality I'm not that familiar with yet. The code looks good

[GitHub] storm pull request: [STORM-831] Adding jira and central logging li...

2015-11-13 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/559#issuecomment-156566901 +1 I think the copyright line should read: ``` Copyright (c) 2015 Github, Inc. ``` But that can be done at merge time. --- If your

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

2015-11-13 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/827#issuecomment-156642074 +1 for adding this. I've done a lot of work with Cassandra in the past (Back when the thrift API was the only game in town) and have always wanted to see

[GitHub] storm pull request: [STORM-831] Adding jira and central logging li...

2015-11-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/559#issuecomment-156217844 The bug icon is licensed free for commercial use, so that should be fine. The statistics icon is MIT licensed by Github, Inc., so we should have an entry

<    1   2   3   4   5   6   7   >