[GitHub] storm pull request: STORM-499

2014-09-30 Thread ptgoetz
GitHub user ptgoetz opened a pull request: https://github.com/apache/storm/pull/274 STORM-499 This simply removes shaded/relocated artifacts from the published POM and promotes transitive dependencies. You can merge this pull request into a Git repository by running: $ git

[GitHub] storm pull request: Change clojure class called by `storm repl`

2014-09-30 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/263#issuecomment-57414742 +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-439: Replace purl.js with jquery url plu...

2014-09-30 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/242#issuecomment-57415388 +1 If no one minds, I'd like to merge this one so I can verify the licensing is correct (no code changed). --- If your project is set up for it, you can reply

[GitHub] storm pull request: STORM-495: KafkaSpout retries with exponential...

2014-10-01 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/254#discussion_r18316629 --- Diff: external/storm-kafka/src/jvm/storm/kafka/SpoutConfig.java --- @@ -26,8 +26,19 @@ public Integer zkPort = null; public String zkRoot

[GitHub] storm pull request: STORM-495: KafkaSpout retries with exponential...

2014-10-01 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/254#discussion_r18323164 --- Diff: external/storm-kafka/src/jvm/storm/kafka/SpoutConfig.java --- @@ -26,8 +26,19 @@ public Integer zkPort = null; public String zkRoot

[GitHub] storm pull request: STORM-514: Refer to post-graduation Storm webs...

2014-10-02 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/277#issuecomment-57593933 +1 Thanks @miguno! --- 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-519 adding tuple as an input param to HB...

2014-10-07 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/283#issuecomment-58234789 +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-488: Exit with 254 error code if storm C...

2014-10-07 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/278#issuecomment-58240854 +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 README.md

2014-10-24 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/292#issuecomment-60419239 +1 Thanks for fixing 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-513 check heartbeat from multilang subpr...

2014-10-24 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/286#issuecomment-60438969 Since there were additional commits added to the pull request, we need to give it more time for others to review before merging, but I am still +1 for the patch

[GitHub] storm pull request: STORM-535:setup 'java.library.path' for native...

2014-11-05 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/298#issuecomment-61857797 -1 The storm platform itself does not require any native libraries, and the config option should be sufficient. --- If your project is set up for it, you can

[GitHub] storm pull request: STORM-378,SleepSpoutWaitStrategy.emptyEmit sho...

2014-11-05 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/295#issuecomment-61860848 -1 I agree with @HeartSaVioR that if we want an implementation of `ISpoutWaitStrategy` that takes into account the `streak` parameter, it should be a separate

[GitHub] storm pull request: [STORM-537] A worker reconnects infinitely to ...

2014-11-05 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/304#issuecomment-61883299 +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-492 with reverting previous merge

2014-11-05 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/308#issuecomment-61897392 Thanks @HeartSaVioR. No need to apologize, we all make mistakes. The important part is that we have a community to review and catch them. When we do. :) I'm fine

[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...

2014-11-05 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-61919968 @HeartSaVioR sorry for the delay. I will test and reply when I have the chance. I would encourage others to do the same as well. --- If your project is set up

[GitHub] storm pull request: use configured local hostname for reporting me...

2014-11-07 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/306#issuecomment-6089 +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-216: Added Authentication and Authorizat...

2014-11-13 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/121#issuecomment-62946641 @revans2 Yeah, I created that branch earlier today then got distracted by meetings before I could send out a notification. That branch is currently identical to master

[GitHub] storm pull request: Storm555: Storm json response should set chars...

2014-11-14 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/313#issuecomment-63092518 +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-329 : buffer message in client and recon...

2014-11-14 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/268#issuecomment-63127132 I tested this by applying it to the 0.9.3 branch and found problems with the unit tests (never-ending cycle of zookeeper reconnects, tests never complete

[GitHub] storm pull request: STORM-513 check heartbeat from multilang subpr...

2014-11-14 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/286#issuecomment-63134028 +1 (again) @harshach The problem you saw was due to two of the `storm.rb` files being out of sync. I will correct that at merge time. --- If your project is set

[GitHub] storm pull request: [STORM-533] Added in client and server IConnec...

2014-11-14 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/302#issuecomment-63138277 @revans2 I think we can merge this and open a JIRA for discussing changing the metrics infrastructure. --- If your project is set up for it, you can reply to this email

[GitHub] storm pull request: STORM-558 change swap! to reset! to fix as...

2014-11-17 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/315#issuecomment-63318696 +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: Storm555: Storm json response should set chars...

2014-11-17 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/313#issuecomment-63367628 Thanks @Parth-Brahmbhatt. I merged this into the master and 0.9.3 branches. --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] storm pull request: STORM-558 change swap! to reset! to fix as...

2014-11-17 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/315#issuecomment-63373190 Thanks @xiaokang. I merged this into the master and 0.9.3 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-567: Move Storm Documentation/Website fr...

2014-11-26 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/330#issuecomment-64701658 I should also mention the site is statically generated using [jekyll](http://jekyllrb.com). First install jekyll (assuming you have ruby installed

[GitHub] storm pull request: STORM-586: TridentKafkaEmitter should catch up...

2014-12-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/339#issuecomment-66829121 +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: typo

2014-12-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/333#issuecomment-66829256 +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: Performance improvement: Fix for https://issue...

2014-12-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/343#issuecomment-66830696 +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-577:supervisor heartbeat and handle ev...

2014-12-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/332#issuecomment-66833294 +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-495: KafkaSpout retries with exponential...

2014-12-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/254#issuecomment-66837722 +1 (again). Looks good to me. --- 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-656: Document external modules and Co...

2015-02-03 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/416#issuecomment-72779670 @HeartSaVioR, yes. Basically just an indicator that at least one committer (i.e. someone with commit rights) is interested in supporting a module. But your

[GitHub] storm pull request: STORM-656: Document external modules and Co...

2015-02-03 Thread ptgoetz
GitHub user ptgoetz opened a pull request: https://github.com/apache/storm/pull/416 STORM-656: Document external modules and Committer Sponsors JIRA: https://issues.apache.org/jira/browse/STORM-656 You can merge this pull request into a Git repository by running: $ git pull

[GitHub] storm pull request: Storm 631: refactoring kafka connector code.

2015-02-02 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/387#issuecomment-72551762 STORM-650 created. Let's try to focus discussion/efforts there. --- 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-637: Integrate PartialKeyGrouping into s...

2015-02-03 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/404#issuecomment-72723482 LGTM. +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

[GitHub] storm pull request: update bylaws for adoption discussion

2015-02-05 Thread ptgoetz
GitHub user ptgoetz opened a pull request: https://github.com/apache/storm/pull/419 update bylaws for adoption discussion Please see [DISCUSS] thread on d...@storm.apache.com NOTE: This pull request should not be merged until a successful VOTE for adoption has taken place

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24037306 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcBolt.java --- @@ -0,0 +1,81 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: STORM-130: Supervisor getting killed due to ja...

2015-02-03 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/401#issuecomment-72725245 +1. I'd also like to see this back-ported to the 0.9.3 branch, but that shouldn't block this from getting merged to master. --- If your project is set up for it, you can

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24037432 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcBolt.java --- @@ -0,0 +1,81 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24039341 --- Diff: external/storm-jdbc/pom.xml --- @@ -0,0 +1,125 @@ +?xml version=1.0 encoding=UTF-8? +!-- + Licensed to the Apache Software Foundation

[GitHub] storm pull request: STORM-640. Storm UI vulnerable to poodle attac...

2015-02-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/412#issuecomment-74145215 +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-658:when config topology.acker.executors...

2015-02-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/417#issuecomment-74128755 -1. This breaks trident functionality if `topology.acker.executors` is `null` in `storm.yaml` and not overridden in the topology conf. I've not dug too far

[GitHub] storm pull request: Storm-539. Storm hive bolt and trident state.

2015-02-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/350#issuecomment-74159664 +1 You should probably list yourself as a committer sponsor. ;) (You can add me as well if you like.) --- If your project is set up for it, you can reply

[GitHub] storm pull request: STORM-652. Use latest junit 4.11 .

2015-02-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/409#issuecomment-74145938 +1 The upgrade does not seem to cause any issues with tests. --- 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-633. Nimbus - HTTP Error 413 full HEAD i...

2015-02-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/393#issuecomment-74160282 +1 (since this is just a doc update, we probably don't need to wait for additional review) --- If your project is set up for it, you can reply to this email and have

[GitHub] storm pull request: STORM-641. Add total number of topologies to a...

2015-02-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/411#issuecomment-74145467 +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-329: fix cascading Storm failure by impr...

2015-02-18 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/429#issuecomment-74885134 +1 I was able to verify the fix, and am in favor of merging. I'd also like to apply it to the 0.9.x branch as I feel it's an important fix. --- If your project is set up

[GitHub] storm pull request: STORM-130. Supervisor getting killed due to ja...

2015-01-28 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/400#issuecomment-71970386 @harshach One nit: white space changes make the diff harder to read. (Though appending '?w=1' to the URL will force github to ignore white space.) Otherwise

[GitHub] storm pull request: STORM-670: restore java 1.6 compatibility (sto...

2015-02-12 Thread ptgoetz
GitHub user ptgoetz opened a pull request: https://github.com/apache/storm/pull/431 STORM-670: restore java 1.6 compatibility (storm-kafka) Not sure how long we want to cling to 1.6 compatibility, but this is a very small change. You can merge this pull request into a Git

[GitHub] storm pull request: Storm-649: Storm HDFS test topologies should w...

2015-02-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/426#issuecomment-74098599 +1, looks fine. --- 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] storm pull request: STORM-581. Add rebalance params to Storm REST ...

2015-02-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/415#issuecomment-74114183 +1 The typo fix is a non-code change so I'm fine with including that. --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] storm pull request: STORM-648. storm examples BasicDRPCTopology fa...

2015-02-13 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/408#issuecomment-74317241 +1 I think this will just prevent stacktraces due to the immediate shutdown, but I'm fine with the change. --- If your project is set up for it, you can reply

[GitHub] storm pull request: STORM-693: when bolt fails to write tuple, it ...

2015-03-18 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/453#issuecomment-83011336 Merged to 0.9.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

[GitHub] storm pull request: Storm-166: Nimbus HA design doc and implementa...

2015-03-19 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/354#issuecomment-83850291 +1 I'll proceed with crating the necessary 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-166: Nimbus HA design doc and implementa...

2015-03-20 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/354#issuecomment-84231994 **NOTE:** 0.11.x-branch has been renamed to 'nimbus-ha-branach'. --- 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-691 Add basic lookup / persist bolts

2015-03-19 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/451#issuecomment-83732719 +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-714] Make CSS more consistent with self...

2015-03-20 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/472#issuecomment-84066145 +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-166: Nimbus HA design doc and implementa...

2015-03-20 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/354#issuecomment-84153349 Thanks @Parth-Brahmbhatt. I've merged this to 0.11.x-branch. --- 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-634: Storm serialization changed to thri...

2015-03-12 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/414#issuecomment-78618488 @revans2 I agree that it would be great to test forward/backward compatibility, but testing that in an automated way would be really difficult since it would likely

[GitHub] storm pull request: STORM-731. Support compiling with HBase-1.0.0.

2015-03-29 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/490#issuecomment-87551582 +1, though it's not necessary because you are a committer. If you can apply the patch, even if you have to do so manually, without violating the spirit of it (yes, I

[GitHub] storm pull request: STORM-563. Kafka Spout doesn't pick up from th...

2015-03-31 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/493#discussion_r27512587 --- Diff: external/storm-kafka/README.md --- @@ -120,6 +120,23 @@ spoutConf.scheme = new SchemeAsMultiScheme(new StringScheme()); OpaqueTridentKafkaSpout

[GitHub] storm pull request: Storm-166: Nimbus HA design doc and implementa...

2015-02-27 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/354#issuecomment-76474553 I'm +1 for merging this, however this feature is targeted for a post-0.10.0 (AKA security release). I'd like to propose that this be merged to a feature branch

[GitHub] storm pull request: STORM-682: supervisor should handle worker sta...

2015-03-05 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/437#issuecomment-77394892 We may want to consider applying this to the 0.9.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

[GitHub] storm pull request: STORM-682: supervisor should handle worker sta...

2015-03-05 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/437#issuecomment-77394741 +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-675. Allow users to have storm-env.sh un...

2015-03-05 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/434#issuecomment-77397949 +1. The diffs of `storm` vs. `storm.py` are minimal and related to windows compatibility. For easy reference, here they are: ``` $ diff -w storm

[GitHub] storm pull request: STORM-675. Allow users to have storm-env.sh un...

2015-03-05 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/434#discussion_r25876670 --- Diff: bin/storm --- @@ -1,5 +1,7 @@ -#!/usr/bin/python - +#!/bin/bash +# +# Copyright 2014 The Apache Software Foundation --- End

[GitHub] storm pull request: STORM-675. Allow users to have storm-env.sh un...

2015-03-05 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/434#discussion_r25876751 --- Diff: conf/storm-env.sh --- @@ -0,0 +1,26 @@ +#!/bin/bash +# +# Copyright 2014 The Apache Software Foundation --- End diff -- Same

[GitHub] storm pull request: STORM-693: when bolt fails to write tuple, it ...

2015-03-05 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/453#issuecomment-77415797 +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-496. task.clj missing debug for logging ...

2015-03-05 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/446#issuecomment-77394266 +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-329: fix cascading Storm failure by impr...

2015-02-23 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/429#issuecomment-75631663 Disregard last message. It was a merge mistake (picked right when I should have picked left). All tests are passing now. --- If your project is set up

[GitHub] storm pull request: STORM-130: Supervisor getting killed due to ja...

2015-02-24 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/418#issuecomment-75842588 merged to 0.9.3-branch. The next release of that branch will be 0.9.4, I'll update JIRA. @harshach Can you close this PR? --- If your project is set up

[GitHub] storm pull request: STORM-329: fix cascading Storm failure by impr...

2015-02-25 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/429#issuecomment-76072022 Patch ported and merged to 0.9.x branch. Benchmarked several topologies (both core and Trident) before (0.9.3) and after (0.9.4-SNAPSHOT) this patch and found

[GitHub] storm pull request: STORM-329: fix cascading Storm failure by impr...

2015-02-24 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/429#issuecomment-75891084 @danielschonfeld Soon. ;) We are probably a handful of weeks out, hopefully less. If you have the resources, testing an RC and reporting back is the best

[GitHub] storm pull request: STORM-329: fix cascading Storm failure by impr...

2015-02-24 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/429#issuecomment-75867311 I finally got this successfully back ported to the 0.9.x branch, with all tests passing. I will merge that soon after more testing and update all associated JIRAs

[GitHub] storm pull request: STORM-329: fix cascading Storm failure by impr...

2015-02-24 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/429#issuecomment-75879324 @miguno yep, that's what I meant. --- 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-329: fix cascading Storm failure by impr...

2015-02-24 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/429#issuecomment-75879629 @danielschonfeld No worries. There will be a 0.9.4 release that includes this fix. No need to wait for 0.10.0. --- If your project is set up for it, you can reply

[GitHub] storm pull request: [STORM-483] extlib for external, extlib-daemon...

2015-04-22 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/516#issuecomment-95337405 +1 (and I assume @revans2 feels the same and just forgot to comment as such :) ). I would suggest that we open a ticket to document this for end users since

[GitHub] storm pull request: [STORM-745] fix storm.cmd to evaluate 'shift' ...

2015-04-28 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/506#issuecomment-97262501 This is a regression that affects 0.9.4, and I've confirmed this patch fixes it. I'm +1 for merging as well as applying it to the 0.9.x branch and releasing

[GitHub] storm pull request: [STORM-796] Add support for error command in...

2015-04-29 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/533#issuecomment-97610607 +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-790 Log task is null instead of let wo...

2015-04-29 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/527#issuecomment-97610139 +1 @HeartSaVioR I'll ad this to the list of patches for 0.9.5 --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] storm pull request: [STORM-796] Add support for error command in...

2015-05-07 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/533#issuecomment-99924195 Thanks @amontalenti. I merged this into master, 0.10.x-branch, and 0.9.x-branch. --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] storm pull request: STORM-561: Add flux as an external module

2015-05-06 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/546#issuecomment-99629737 @HeartSaVioR, Thanks for the review! Thanks for catching that... I will fix it. --- If your project is set up for it, you can reply to this email

[GitHub] storm pull request: STORM-561: Add flux as an external module

2015-05-06 Thread ptgoetz
GitHub user ptgoetz opened a pull request: https://github.com/apache/storm/pull/546 STORM-561: Add flux as an external module For a description of everything this does, it's probably easiest to look at the README file: https://github.com/ptgoetz/storm/blob/STORM-561

[GitHub] storm pull request: STORM-561: Add flux as an external module

2015-05-07 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/546#issuecomment-99889582 Thanks again for the review @HeartSaVioR. I've addressed all your comments. --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] storm pull request: [STORM-745] fix storm.cmd to evaluate 'shift' ...

2015-05-07 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/506#issuecomment-99911140 Thanks for the patch @HeartSaVioR. I merged this to master, 0.9.x-branch, and 0.10.x-branch. --- If your project is set up for it, you can reply to this email and have

[GitHub] storm pull request: [STORM-789] Send more topology context to Mult...

2015-05-15 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/525#issuecomment-102474003 merged to 0.10.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

[GitHub] storm pull request: STORM-750: Set Config serialVersionUID

2015-04-06 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/507#issuecomment-90165302 Did this just go from pull request to merged in less than 15 minutes? I know it's a very minor change, and has all the requisite +1s, but that seems a little

[GitHub] storm pull request: STORM-750: Set Config serialVersionUID

2015-04-06 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/507#issuecomment-90211059 @revans2, True, we can retroactively revert if a -1 comes in, but the bylaws state that the minimum approval time for a code change is 1 day. That's

[GitHub] storm pull request: Storm 748 - Package Multi-Lang scripts so they...

2015-04-06 Thread ptgoetz
GitHub user ptgoetz opened a pull request: https://github.com/apache/storm/pull/509 Storm 748 - Package Multi-Lang scripts so they don't have to be duplicated This moves storm's multi-lang components to a single location so they can be referenced from, rather than copied to other

[GitHub] storm pull request: STORM-748: Package Multi-Lang scripts so they ...

2015-04-06 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/509#issuecomment-90279252 Commenting to trigger JIRA sync. Ignore. --- 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-563. Kafka Spout doesn't pick up from th...

2015-04-06 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/493#issuecomment-90313220 It wouldn't hurt to expand on what `System.currentTimeMillis()` means in that context (i.e. if you have a specific time stored in epoch format, you can start from

[GitHub] storm pull request: STORM-750: Set Config serialVersionUID

2015-04-06 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/507#issuecomment-90296417 @knusbaum I agree. Seems like an AND vs. OR issue between those two sentences. IMO it should be AND. To me, it seems only fair to allow the sun to pass

[GitHub] storm pull request: STORM-563. Kafka Spout doesn't pick up from th...

2015-04-06 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/493#issuecomment-90320879 That being said, I'm +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

[GitHub] storm pull request: STORM-757: Simulated time can leak out on erro...

2015-04-06 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/508#issuecomment-90307502 Nice 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

[GitHub] storm pull request: STORM-748: Package Multi-Lang scripts so they ...

2015-04-06 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/509#issuecomment-90306940 @HeartSaVioR Thanks for the feedback. I'm open to either approach... My thought behind keeping them separate was that some languages may require compilation (e.g

[GitHub] storm pull request: STORM-694: java.lang.ClassNotFoundException: b...

2015-04-07 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/492#issuecomment-90661810 +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-752] [storm-redis] Clarify Redis*StateU...

2015-05-19 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/488#issuecomment-103519540 +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-737] Check task-node+port with read lo...

2015-05-20 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/557#issuecomment-103928314 Testing with #521 applied to 0.10.x-branch I'm actually seeing a performance ***improvement***. With core storm topologies there's an increase in throughput

[GitHub] storm pull request: [STORM-737] Check task-node+port with read lo...

2015-05-19 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/557#issuecomment-103668907 So far I think I'm seeing a performance regression in terms of throughput, but I want to let all the test cases run to be sure. I will have more information tomorrow

[GitHub] storm pull request: [STORM-737] Check task-node+port with read lo...

2015-05-20 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/557#issuecomment-103992667 @HeartSaVioR, #521 passed my fault tolerance test (which randomly kills workers and tests for data loss). I'd suggest closing this pull request

[GitHub] storm pull request: Storm-818: storm-eventhubs configuration impro...

2015-05-20 Thread ptgoetz
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/552#issuecomment-104067426 +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   >