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

2016-12-07 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1800 One more option, still ugly, but with a lot less impact. I can do something similar to Hadoop. They use several different invocations of the maven assembly plugin into directories (predates

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

2016-12-07 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1800 So shading is not really an option for jersey. I was able to split the DRPC server off into its own package with tests, but packaging it up with the assembly plugin is proving to be difficult.

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

2016-12-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91354645 --- 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-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91367220 --- 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 issue #1807: fix NullPointException with acked.get(rtp)

2016-12-07 Thread srdo
Github user srdo commented on the issue: https://github.com/apache/storm/pull/1807 @cutd Great. Could you update the PR so it changes `fail` instead of `doSeekRetriableTopicPartitions`? You might also want to create an issue at https://issues.apache.org/jira/browse/STORM so this fix

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

2016-12-07 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1800 @revans2 Okay. Let me review the packaging changes. --- 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 #1406: [STORM-433] [WIP] Executor queue backlog metric

2016-12-07 Thread erikdw
Github user erikdw commented on the issue: https://github.com/apache/storm/pull/1406 @abhishekagarwal87 & @HeartSaVioR : any update on this? Getting visibility into queue depth in storm is a major reason for us to even consider upgrading off of the storm-0.9.x branch. Do you need

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

2016-12-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91348095 --- 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-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91349569 --- 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-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91348211 --- 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-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91347833 --- 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-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91348040 --- 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-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91350132 --- 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-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91352188 --- Diff: external/storm-kafka-client/README.md --- @@ -1,90 +1,222 @@ -#Storm Kafka Spout with New Kafka Consumer API +#Storm Kafka integration using

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

2016-12-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91346392 --- 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-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91351636 --- Diff: examples/storm-kafka-client-examples/src/main/java/org/apache/storm/kafka/trident/TridentKafkaClientWordCountNamedTopics.java --- @@ -18,87 +18,48 @@

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

2016-12-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91345785 --- 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-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91353717 --- 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 pull request #1808: STORM-2225: change spout config to be simpler.

2016-12-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91359755 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java --- @@ -290,8 +285,13 @@ private boolean

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

2016-12-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91363994 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/NamedSubscription.java --- @@ -0,0 +1,66 @@ +/* + * Licensed to the

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

2016-12-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91366137 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/trident/KafkaTridentSpoutEmitter.java --- @@ -152,7 +152,9 @@ private long

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

2016-12-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91361471 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutConfig.java --- @@ -60,123 +68,197 @@ * If no offset has

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

2016-12-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91353321 --- 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 pull request #1808: STORM-2225: change spout config to be simpler.

2016-12-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91364347 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/RecordTranslator.java --- @@ -0,0 +1,59 @@ +/* + * Licensed to the

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

2016-12-07 Thread srdo
Github user srdo commented on the issue: https://github.com/apache/storm/pull/1696 @revans2 ping for review if you have time. I'd like to get this in before too long if possible. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

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

2016-12-07 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1816 Looks like a good start, but it really needs some documentation. It would also be helpful to include a sample model + CSV data, without that it's not very clear how to run the example. --- If your

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

2016-12-07 Thread ptgoetz
Github user ptgoetz commented on the issue: https://github.com/apache/storm/pull/1816 Also, what about unit tests? --- 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

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

2016-12-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91362733 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutConfig.java --- @@ -60,123 +68,197 @@ * If no offset has

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

2016-12-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91362618 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutConfig.java --- @@ -60,123 +68,197 @@ * If no offset has

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

2016-12-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91351118 --- 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-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91351206 --- 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-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91362663 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutConfig.java --- @@ -60,123 +68,197 @@ * If no offset has

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

2016-12-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91360488 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutConfig.java --- @@ -18,34 +18,42 @@ package

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

2016-12-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91355497 --- 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 pull request #1808: STORM-2225: change spout config to be simpler.

2016-12-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91364222 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/RecordTranslator.java --- @@ -0,0 +1,59 @@ +/* + * Licensed to the

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

2016-12-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91365059 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/RecordTranslator.java --- @@ -0,0 +1,59 @@ +/* + * Licensed to the

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

2016-12-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91350461 --- 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-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91366483 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/trident/KafkaTridentSpoutManager.java --- @@ -44,29 +40,30 @@ //

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

2016-12-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91357255 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/bolt/selector/FieldIndexTopicSelector.java --- @@ -0,0 +1,48 @@ +/** + *

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

2016-12-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91356935 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/bolt/mapper/TupleToKafkaMapper.java --- @@ -0,0 +1,32 @@ +/** + * Licensed

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

2016-12-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91365926 --- Diff: external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpout.java --- @@ -185,7 +178,9 @@ private long doSeek(TopicPartition tp,

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

2016-12-07 Thread srdo
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/1808#discussion_r91367555 --- Diff: external/storm-kafka-client/src/test/java/org/apache/storm/kafka/spout/ByTopicRecordTranslatorTest.java --- @@ -0,0 +1,72 @@ +/* + * Licensed

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

2016-12-07 Thread hmcl
Github user hmcl commented on the issue: https://github.com/apache/storm/pull/1816 @vesense I addressed your serialization comment. I had to do some refactoring of the code because it was a strong requirement to enforce `ModelRunner` to be serializable. For instance, the JPMML

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

2016-12-07 Thread csivaguru
Github user csivaguru commented on a diff in the pull request: https://github.com/apache/storm/pull/1816#discussion_r91385214 --- 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 #1816: STORM-2223: PMMLBolt

2016-12-07 Thread vesense
Github user vesense commented on a diff in the pull request: https://github.com/apache/storm/pull/1816#discussion_r91297902 --- Diff: external/storm-pmml/src/main/java/org/apache/storm/pmml/runner/ModelRunner.java --- @@ -0,0 +1,30 @@ +/* + * Licensed to the Apache

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

2016-12-07 Thread vesense
Github user vesense commented on the issue: https://github.com/apache/storm/pull/1816 @hmcl Code looks good. Left one comment. I think it is also necessary to add a README file to let people know how to use it. --- If your project is set up for it, you can reply to this email and

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

2016-12-07 Thread vesense
Github user vesense commented on the issue: https://github.com/apache/storm/pull/1816 @harshach Thanks for your detailed explanations. Yes, this is a good start. I'm +1 for adding this in. This might attract many people who interest NLP, ML. --- If your project is set up for it,

[GitHub] storm pull request #1817: STORM-2237:Nimbus reports bad supervisor heartbeat...

2016-12-07 Thread knusbaum
GitHub user knusbaum opened a pull request: https://github.com/apache/storm/pull/1817 STORM-2237:Nimbus reports bad supervisor heartbeat - unknown version or thrift exception You can merge this pull request into a Git repository by running: $ git pull

[GitHub] storm issue #1815: STORM-2235 Introduce new option: 'add remote repositories...

2016-12-07 Thread priyank5485
Github user priyank5485 commented on the issue: https://github.com/apache/storm/pull/1815 +1 (non-binding) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so,

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

2016-12-07 Thread srdo
Github user srdo commented on the issue: https://github.com/apache/storm/pull/1808 I like the new configuration design. Backward compatibility is not an issue for us. I'm wondering if enough people have even switched to the new spout yet to make backward compatibility for 1.x a must.

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

2016-12-07 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1800 @ptgoetz I have updated the packaging to have a separate directory for the DRPC server dependencies. I have run manual tests and everything works. The big difference is that

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

2016-12-07 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/1808 @srdo for me backwards compatibility for 1.x is more a question of violating out versioning than anything else. --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] storm issue #1817: STORM-2237: Nimbus reports bad supervisor heartbeat - unk...

2016-12-07 Thread knusbaum
Github user knusbaum commented on the issue: https://github.com/apache/storm/pull/1817 I'll have a version up for 2.x as well. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature

[GitHub] storm issue #1815: STORM-2235 Introduce new option: 'add remote repositories...

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