Re: [ANNOUNCE] New Storm Commiter/PMC Member: Xin Wang

2016-12-01 Thread Bobby Evans
Congratulations. - Bobby On Thursday, December 1, 2016, 4:23:16 AM CST, Satish Duggana wrote:Congratulations Xin!! ~Satish. On 12/1/16, 10:54 AM, "Matthias J. Sax" wrote:     -BEGIN PGP SIGNED MESSAGE-     Hash: SHA512         Congrats!  

[GitHub] storm issue #1786: STORM-1281: LocalCluster, testing4j and testing.clj to ja...

2016-12-01 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1786 @ptgoetz I'll look at adding in some javadocs. In general I tried to add javadocs where there were clojure doc strings, but there were not too many of them either. --- If your project is set up

[GitHub] storm pull request #1744: STORM-1276: line for line translation of nimbus to...

2016-12-01 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/1744#discussion_r90464031 --- Diff: storm-core/src/jvm/org/apache/storm/daemon/nimbus/Nimbus.java --- @@ -0,0 +1,3729 @@ +/** + * Licensed to the Apache Software Foundation

[GitHub] storm pull request #1765: STORM-2190: reduce contention between submission a...

2016-12-01 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/1765#discussion_r90465747 --- Diff: examples/storm-starter/src/jvm/org/apache/storm/starter/WordCountTopology.java --- @@ -90,7 +90,9 @@ public static void main(String[] args) throws

[GitHub] storm issue #1808: STORM-2225: change spout config to be simpler.

2016-12-01 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1808 I updated the docs and the examples. This is a non-backwards compatible change from the 1.x release. I have some code (not included here) that can maintain most compatibility if we really want to,

[GitHub] storm issue #1783: STORM-2204 Adding caching capabilities in HBaseLookupBolt

2016-12-01 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1783 I guess guava is OK for 1.x. I would prefer to see it shaded if we do go with Guava, but I am only a -0 if it is not shaded. --- If your project is set up for it, you can reply to this email and

[GitHub] storm pull request #1808: STORM-1997: copy state/bolt from storm-kafka to st...

2016-12-01 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/1808 STORM-1997: copy state/bolt from storm-kafka to storm-kafka-client STORM-2228: removed ability to request a single topic go to multiple streams You can merge this pull request into a Git

[GitHub] storm issue #1783: STORM-2204 Adding caching capabilities in HBaseLookupBolt

2016-12-01 Thread HeartSaVioR
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/1783 OK. Since we have other modules which depends on Guava, it might be better to have a rule and apply all of them. +1 to shade Guava on external modules if possible. --- If your project is set

[GitHub] storm issue #1744: STORM-1276: line for line translation of nimbus to java

2016-12-01 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1744 @erikdw I addressed your nits. I appreciate the nits because we don't have a coding standard yet. If we are just down to nits I really would like to get this in. --- If your project is

[GitHub] storm issue #1807: fix NullPointException with acked.get(rtp)

2016-12-01 Thread srdo
Github user srdo commented on the issue: https://github.com/apache/storm/pull/1807 Good catch. I think I'd prefer disabling the retry logic entirely when auto commit is enabled though. It doesn't really make sense to me that you would want to have Storm guarantee at least once

[GitHub] storm issue #1807: fix NullPointException with acked.get(rtp)

2016-12-01 Thread hmcl
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1807 @srdo @cutd looking at it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes

[GitHub] storm pull request #1809: STORM-1886 Extend KeyValueState iface with delete

2016-12-01 Thread kosii
GitHub user kosii opened a pull request: https://github.com/apache/storm/pull/1809 STORM-1886 Extend KeyValueState iface with delete The same as #1470, just against the 1.x-branch You can merge this pull request into a Git repository by running: $ git pull

[GitHub] storm issue #1470: STORM-1886 Extend KeyValueState iface with delete

2016-12-01 Thread kosii
Github user kosii commented on the issue: https://github.com/apache/storm/pull/1470 @arunmahadevan actually my patch depends on #2020, which isn't yet in 1.x-branch. how should we proceed? --- If your project is set up for it, you can reply to this email and have your reply appear

[GitHub] storm issue #1786: STORM-1281: LocalCluster, testing4j and testing.clj to ja...

2016-12-01 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1786 @ptgoetz I added in javadocs for most of the new classes --- 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

Re: [ANNOUNCE] New Storm Commiter/PMC Member: Xin Wang

2016-12-01 Thread Hugo Da Cruz Louro
Congratulations Xing! Keep up the good work. Best, Hugo > On Dec 1, 2016, at 6:17 AM, Bobby Evans wrote: > > Congratulations. > - Bobby > > On Thursday, December 1, 2016, 4:23:16 AM CST, Satish Duggana > wrote:Congratulations Xin!! > >

[GitHub] storm issue #1744: STORM-1276: line for line translation of nimbus to java

2016-12-01 Thread erikdw
Github user erikdw commented on the issue: https://github.com/apache/storm/pull/1744 @revans2 : Since I didn't have any substantive blocking commits I'm ok with merging it. And it's been long enough that I think you're probably ok to merge away. Any hidden problems will hopefully

[GitHub] storm issue #1470: STORM-1886 Extend KeyValueState iface with delete

2016-12-01 Thread kosii
Github user kosii commented on the issue: https://github.com/apache/storm/pull/1470 Yes, I'll look into that! --- 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

[GitHub] storm issue #1804: STORM-2222: Repeated NPEs thrown in nimbus if rebalance f...

2016-12-01 Thread jerrypeng
Github user jerrypeng commented on the issue: https://github.com/apache/storm/pull/1804 +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

[GitHub] storm pull request #1809: STORM-1886 Extend KeyValueState iface with delete

2016-12-01 Thread kosii
Github user kosii closed the pull request at: https://github.com/apache/storm/pull/1809 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is

[GitHub] storm pull request #1785: [STORM-2201] Add dynamic scheduler configuration l...

2016-12-01 Thread jerrypeng
Github user jerrypeng commented on a diff in the pull request: https://github.com/apache/storm/pull/1785#discussion_r90526318 --- Diff: storm-core/src/jvm/org/apache/storm/Config.java --- @@ -2177,13 +2177,37 @@ @isMapEntryType(keyType = String.class, valueType =

[GitHub] storm issue #1744: STORM-1276: line for line translation of nimbus to java

2016-12-01 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1744 I am merging I am still happy to make changes on follow on JIRA. --- 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

topology.debug always true

2016-12-01 Thread Ohad Edelstein
We see that the topology.debug set to two different values: [cid:57BDBB53-DB37-4DF8-8B26-D9BD1874CB9D] And: [cid:C121FA48-8020-4B00-A6B2-0652B1EF82BE] We set the topology.debug to false ( we know its set that way to default but we thought that, this was the problem ). Is there a different

[GitHub] storm issue #1783: STORM-2204 Adding caching capabilities in HBaseLookupBolt

2016-12-01 Thread ambud
Github user ambud commented on the issue: https://github.com/apache/storm/pull/1783 1.x branch PR https://github.com/apache/storm/pull/1810 --- 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 #1810: STORM-2204 Adding Bolt side caching for HBase Lookup Bolt...

2016-12-01 Thread ambud
Github user ambud commented on the issue: https://github.com/apache/storm/pull/1810 https://github.com/apache/storm/pull/1783 original PR --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this

[GitHub] storm issue #1781: STORM-1369: Add MapState implementation to storm-cassandr...

2016-12-01 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1781 Unfortunately this patch uses Cassandra-unit, which is LGPL licensed, so in it's current state we can't accept it. For that reason I am -1. We've run into this before, and I've lobbied the

[GitHub] storm issue #1795: STORM-2215: validate blobs are present before submitting

2016-12-01 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1795 @HeartSaVioR I rebased and addressed the review comments, but I want to merge in #1744 first and then I will update this one again. --- If your project is set up for it, you can reply to this email

[GitHub] storm issue #1786: STORM-1281: LocalCluster, testing4j and testing.clj to ja...

2016-12-01 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1786 Rebased now that java nimbus is in. --- 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 issue #1744: STORM-1276: line for line translation of nimbus to java

2016-12-01 Thread srdo
Github user srdo commented on the issue: https://github.com/apache/storm/pull/1744 I only really have nitpicks, so I'm good with merging too. Really 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

[GitHub] storm pull request #1744: STORM-1276: line for line translation of nimbus to...

2016-12-01 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1744#discussion_r90513444 --- Diff: storm-core/src/jvm/org/apache/storm/daemon/nimbus/Nimbus.java --- @@ -0,0 +1,3729 @@ +/** + * Licensed to the Apache Software Foundation (ASF)

[GitHub] storm pull request #1744: STORM-1276: line for line translation of nimbus to...

2016-12-01 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1744#discussion_r90521664 --- Diff: storm-core/src/jvm/org/apache/storm/daemon/nimbus/Nimbus.java --- @@ -0,0 +1,3729 @@ +/** + * Licensed to the Apache Software Foundation (ASF)

[GitHub] storm pull request #1744: STORM-1276: line for line translation of nimbus to...

2016-12-01 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1744#discussion_r90522761 --- Diff: storm-core/src/jvm/org/apache/storm/daemon/nimbus/Nimbus.java --- @@ -0,0 +1,3729 @@ +/** + * Licensed to the Apache Software Foundation (ASF)

[GitHub] storm pull request #1744: STORM-1276: line for line translation of nimbus to...

2016-12-01 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1744#discussion_r90522896 --- Diff: storm-core/src/jvm/org/apache/storm/daemon/nimbus/Nimbus.java --- @@ -0,0 +1,3729 @@ +/** + * Licensed to the Apache Software Foundation (ASF)

[GitHub] storm pull request #1744: STORM-1276: line for line translation of nimbus to...

2016-12-01 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1744#discussion_r90510263 --- Diff: storm-core/src/jvm/org/apache/storm/daemon/nimbus/Nimbus.java --- @@ -0,0 +1,3729 @@ +/** + * Licensed to the Apache Software Foundation (ASF)

[GitHub] storm pull request #1810: STORM-2204 Adding Bolt side caching for HBase Look...

2016-12-01 Thread ambud
GitHub user ambud opened a pull request: https://github.com/apache/storm/pull/1810 STORM-2204 Adding Bolt side caching for HBase Lookup Bolt for 1.x branch You can merge this pull request into a Git repository by running: $ git pull https://github.com/ambud/storm 1.x-branch

[GitHub] storm pull request #1744: STORM-1276: line for line translation of nimbus to...

2016-12-01 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/1744 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is

[GitHub] storm issue #1783: STORM-2204 Adding caching capabilities in HBaseLookupBolt

2016-12-01 Thread ambud
Github user ambud commented on the issue: https://github.com/apache/storm/pull/1783 Can we merge this? @HeartSaVioR @revans2 @vesense I will need to open another pull request for 1.x branch. --- If your project is set up for it, you can reply to this email and have your

[GitHub] storm issue #1767: STORM-2194: Report error and die, not report error or die

2016-12-01 Thread sathyafmt
Github user sathyafmt commented on the issue: https://github.com/apache/storm/pull/1767 @revans2 - His change in storm-1.0.2 is in the report-error-and-die function, so it should shutdown, correct ? --- If your project is set up for it, you can reply to this email and have your

[GitHub] storm pull request #1764: STORM-2190: reduce contention between submission a...

2016-12-01 Thread jerrypeng
Github user jerrypeng commented on a diff in the pull request: https://github.com/apache/storm/pull/1764#discussion_r90540863 --- Diff: storm-core/src/clj/org/apache/storm/daemon/nimbus.clj --- @@ -1008,23 +1008,24 @@ (reset! (:id->worker-resources nimbus) {}))

[GitHub] storm pull request #1764: STORM-2190: reduce contention between submission a...

2016-12-01 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/1764#discussion_r90542812 --- Diff: storm-core/src/clj/org/apache/storm/daemon/nimbus.clj --- @@ -1008,23 +1008,24 @@ (reset! (:id->worker-resources nimbus) {})) ;;

[GitHub] storm issue #1800: STORM-2217: Make DRPC pure java

2016-12-01 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1800 Rebased again --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if

[GitHub] storm issue #1768: STORM-2194: Report error and die, not report error or die

2016-12-01 Thread chawco
Github user chawco commented on the issue: https://github.com/apache/storm/pull/1768 I've left some follow up comments in [1767](https://github.com/apache/storm/pull/1767). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as

[GitHub] storm issue #1800: STORM-2217: Make DRPC pure java

2016-12-01 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1800 @ptgoetz if this looks good as is I will look into shading Jersey. --- 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 issue #1764: STORM-2190: reduce contention between submission and sche...

2016-12-01 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1764 Just so we don't miss the comment from @jerrypeng > Couldn't a wrong ordering of events happen since we are locking when calculating a scheduling then unlocking and then locking and

[GitHub] storm issue #1786: STORM-1281: LocalCluster, testing4j and testing.clj to ja...

2016-12-01 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1786 @revans2 Thanks. Still +1. (One procedural nit: Rebasing/squashing on big PRs like this can make reviews a little harder. E.g. After you added the javadocs I wanted to review just that

[GitHub] storm issue #1767: STORM-2194: Report error and die, not report error or die

2016-12-01 Thread chawco
Github user chawco commented on the issue: https://github.com/apache/storm/pull/1767 So, there's some ambiguity in place here that would need to get resolved. Basically, there's two sources for InterruptedException/InterruptedIOException -- one from Storm itself, and one from the

[GitHub] storm pull request #1765: STORM-2190: reduce contention between submission a...

2016-12-01 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/1765#discussion_r90534501 --- Diff: examples/storm-starter/src/jvm/org/apache/storm/starter/WordCountTopology.java --- @@ -90,7 +90,9 @@ public static void main(String[] args) throws

[GitHub] storm pull request #1764: STORM-2190: reduce contention between submission a...

2016-12-01 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/1764#discussion_r90538639 --- Diff: storm-core/src/clj/org/apache/storm/daemon/nimbus.clj --- @@ -1008,23 +1008,24 @@ (reset! (:id->worker-resources nimbus) {})) ;;

[GitHub] storm issue #1584: STORM-1992 Added artifacts to make storm.js into an npm p...

2016-12-01 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1584 My apologies for letting this go without review for so long. +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

[GitHub] storm pull request #1798: Storm-2203 Add a getAll method to KeyValueState in...

2016-12-01 Thread aandis
Github user aandis commented on a diff in the pull request: https://github.com/apache/storm/pull/1798#discussion_r90398126 --- Diff: storm-core/src/jvm/org/apache/storm/state/KeyValueState.java --- @@ -45,4 +47,9 @@ * @return the value or defaultValue if no mapping is

[GitHub] storm pull request #1798: Storm-2203 Add a getAll method to KeyValueState in...

2016-12-01 Thread aandis
Github user aandis commented on a diff in the pull request: https://github.com/apache/storm/pull/1798#discussion_r90405070 --- Diff: storm-core/src/jvm/org/apache/storm/state/KeyValueState.java --- @@ -45,4 +47,9 @@ * @return the value or defaultValue if no mapping is

[GitHub] storm issue #1742: STORM-2170 [Storm SQL] Add built-in socket datasource to ...

2016-12-01 Thread vesense
Github user vesense commented on the issue: https://github.com/apache/storm/pull/1742 +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 #1798: Storm-2203 Add a getAll method to KeyValueState in...

2016-12-01 Thread arunmahadevan
Github user arunmahadevan commented on a diff in the pull request: https://github.com/apache/storm/pull/1798#discussion_r90406042 --- Diff: storm-core/src/jvm/org/apache/storm/state/KeyValueState.java --- @@ -45,4 +47,9 @@ * @return the value or defaultValue if no mapping

[GitHub] storm pull request #1798: Storm-2203 Add a getAll method to KeyValueState in...

2016-12-01 Thread aandis
Github user aandis commented on a diff in the pull request: https://github.com/apache/storm/pull/1798#discussion_r90408316 --- Diff: storm-core/src/jvm/org/apache/storm/state/KeyValueState.java --- @@ -45,4 +47,9 @@ * @return the value or defaultValue if no mapping is

[GitHub] storm issue #1783: STORM-2204 Adding caching capabilities in HBaseLookupBolt

2016-12-01 Thread vesense
Github user vesense commented on the issue: https://github.com/apache/storm/pull/1783 +1 for merging to master after updating `docs/storm-hbase.md`. `external/storm-hbase/README.md` has been documented. --- If your project is set up for it, you can reply to this email and have your

[GitHub] storm pull request #1798: Storm-2203 Add a getAll method to KeyValueState in...

2016-12-01 Thread HeartSaVioR
Github user HeartSaVioR commented on a diff in the pull request: https://github.com/apache/storm/pull/1798#discussion_r90406354 --- Diff: storm-core/src/jvm/org/apache/storm/state/KeyValueState.java --- @@ -45,4 +47,9 @@ * @return the value or defaultValue if no mapping

Re: [ANNOUNCE] New Storm Commiter/PMC Member: Xin Wang

2016-12-01 Thread Satish Duggana
Congratulations Xin!! ~Satish. On 12/1/16, 10:54 AM, "Matthias J. Sax" wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA512 Congrats! On 11/30/16 8:43 PM, Jungtaek Lim wrote: > Please join me in welcoming Xin Wang as a new Apache Storm >

Re: [ANNOUNCE] New Storm Commiter/PMC Member: Xin Wang

2016-12-01 Thread Xin Wang
Thanks everyone! It is really awesome to be working with you on Storm! - Xin Wang 2016-12-02 0:42 GMT+08:00 Hugo Da Cruz Louro : > Congratulations Xing! Keep up the good work. > Best, > Hugo > > > On Dec 1, 2016, at 6:17 AM, Bobby Evans >

[GitHub] storm issue #1470: STORM-1886 Extend KeyValueState iface with delete

2016-12-01 Thread arunmahadevan
Github user arunmahadevan commented on the issue: https://github.com/apache/storm/pull/1470 @kosii can you cherry pick the commits from 1606 plus your commits and raise a pr for 1.x branch? --- If your project is set up for it, you can reply to this email and have your reply appear