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

2016-12-08 Thread hmcl
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1808 @revans2 I would like to take a look at this before this gets merged 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

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

2016-12-08 Thread srdo
Github user srdo commented on the issue: https://github.com/apache/storm/pull/1808 I think my only remaining nit is the spelling error in KafkaBolt. Thanks @revans2. I am +1 on this. --- If your project is set up for it, you can reply to this email and have your reply appear on

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

2016-12-08 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91622857 --- Diff: external/storm-kafka-client/src/test/java/org/apache/storm/kafka/bolt/KafkaBoltTest.java --- @@ -0,0 +1,91 @@ +/** + * Licensed to the Apache

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

2016-12-08 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91621754 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/bolt/KafkaBolt.java --- @@ -0,0 +1,194 @@ +/** + * Licensed to the Apache

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

2016-12-08 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1808 @srdo I think I addressed all of your review comments. --- 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 #1808: STORM-2225: change spout config to be simpler.

2016-12-08 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91618249 --- Diff: external/storm-kafka-client/src/test/java/org/apache/storm/kafka/bolt/KafkaBoltTest.java --- @@ -0,0 +1,91 @@ +/** + * Licensed to the

[GitHub] storm pull request #1816: STORM-2223: PMMLBolt

2016-12-08 Thread csivaguru
Github user csivaguru commented on a diff in the pull request: https://github.com/apache/storm/pull/1816#discussion_r91615776 --- Diff: external/storm-pmml/src/main/java/org/apache/storm/pmml/runner/jpmml/JpmmlFactory.java --- @@ -0,0 +1,88 @@ +/* + * Licensed to the

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

2016-12-08 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91613603 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/bolt/KafkaBolt.java --- @@ -0,0 +1,194 @@ +/** + * Licensed to the Apache

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

2016-12-08 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1781 @mkoch1 Thanks for addressing the license issue. The way you handled the borrowed code looks fine, the important part is to leave the original copyright statement, which you did. Thanks for pointing

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

2016-12-08 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91592134 --- Diff: docs/storm-kafka-client.md --- @@ -1,90 +1,222 @@ -#Storm Kafka Spout with New Kafka Consumer API +#Storm Kafka integration using the

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

2016-12-08 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91592178 --- Diff: docs/storm-kafka-client.md --- @@ -1,90 +1,222 @@ -#Storm Kafka Spout with New Kafka Consumer API +#Storm Kafka integration using the

[GitHub] storm issue #1816: STORM-2223: PMMLBolt

2016-12-08 Thread harshach
Github user harshach commented on the issue: https://github.com/apache/storm/pull/1816 @hmcl I am +1 on merging. I would like to see the InputStream option added to 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

[GitHub] storm issue #1818: STORM-2104 1.x

2016-12-08 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1818 +1 pending travis (and the waiting period) --- 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 #1816: STORM-2223: PMMLBolt

2016-12-08 Thread ptgoetz
Github user ptgoetz commented on a diff in the pull request: https://github.com/apache/storm/pull/1816#discussion_r91586225 --- Diff: external/storm-pmml/src/main/java/org/apache/storm/pmml/runner/jpmml/JpmmlFactory.java --- @@ -0,0 +1,88 @@ +/* + * Licensed to the Apache

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

2016-12-08 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1800 Okay, that last commit answered a lot of questions I had regarding packaging. ;) +1 I was able to build a distribution, unpack it, and run the drpc service. --- If your project is set up

[GitHub] storm pull request #1818: STORM-2104 1.x

2016-12-08 Thread srdo
GitHub user srdo opened a pull request: https://github.com/apache/storm/pull/1818 STORM-2104 1.x 1.x version of https://github.com/apache/storm/pull/1696 You can merge this pull request into a Git repository by running: $ git pull https://github.com/srdo/storm STORM-2104-1.x

[GitHub] storm pull request #1696: STORM-2104: More graceful handling of acked/failed...

2016-12-08 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/1696 --- 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 #1696: STORM-2104: More graceful handling of acked/failed tuples...

2016-12-08 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1696 This looks good to me. Now that I have gone through the kafka spout code for my other pull request I am confident in giving this a +1. --- If your project is set up for it, you can reply to this

[GitHub] storm pull request #1696: STORM-2104: More graceful handling of acked/failed...

2016-12-08 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/1696#discussion_r91569248 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/SerializableDeserializer.java --- @@ -0,0 +1,25 @@ +/* + *

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

2016-12-08 Thread srdo
Github user srdo commented on the issue: https://github.com/apache/storm/pull/1808 Sure, that makes sense. Maybe storm-kafka-client should have been marked unstable since it's fairly new, it's still in a phase where the kinks are being worked out, even if the version number says

[GitHub] storm pull request #1815: STORM-2235 Introduce new option: 'add remote repos...

2016-12-08 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/1815 --- 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 #1674: STORM-2083: Blacklist scheduler

2016-12-08 Thread nilday
Github user nilday commented on the issue: https://github.com/apache/storm/pull/1674 As soon as I find out that nimbus has been transferred to Java, I start to work on this again. The blacklist scheduler has been working on our production environment(Storm v1.0.1) for several months