Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/361#issuecomment-98820091
+1 looks great. Thanks for doing 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
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/361
---
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 user curtisallen commented on the pull request:
https://github.com/apache/storm/pull/361#issuecomment-97934446
@revans2 now we throw NPEs if the values are missing. Leveraged guava's
`Preconditions` . Let me know if you need anything else. Thanks!
---
If your project is set
Github user curtisallen commented on the pull request:
https://github.com/apache/storm/pull/361#issuecomment-97487810
@revans2 Thanks for looking at this. Before we didn't throw any exceptions
when the config was incorrect. I added this
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/361#issuecomment-97512345
@curtisallen I am not really concerned about it being an NPE, or a
RuntimeException, my concern is two fold.
First If we mis configure something like Config we
Github user curtisallen commented on the pull request:
https://github.com/apache/storm/pull/361#issuecomment-97513938
@revans2 agreed, the concerns you brought make a lot of sense to me. I
should have some time later this week to fail fast ;)
---
If your project is set up for it,
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/361#issuecomment-97525358
@curtisallen Thanks. If you could post a comment here when it is updated
I'll take a look and we should hopefully be able to get it in soon.
---
If your project is set
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/361#issuecomment-97201701
@curtisallen Sorry it has taken so long to review this. I think the
changes look OK. I am curious why we only logging an error instead of throwing
an exception.
Github user curtisallen commented on a diff in the pull request:
https://github.com/apache/storm/pull/361#discussion_r22474338
--- Diff:
external/storm-kafka/src/jvm/storm/kafka/DynamicBrokersReader.java ---
@@ -41,6 +41,14 @@
private String _topic;
public
Github user curtisallen commented on a diff in the pull request:
https://github.com/apache/storm/pull/361#discussion_r22474024
--- Diff:
external/storm-kafka/src/jvm/storm/kafka/DynamicBrokersReader.java ---
@@ -149,4 +158,19 @@ private Broker getBrokerHost(byte[] contents) {
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/361#discussion_r22435307
--- Diff:
external/storm-kafka/src/jvm/storm/kafka/DynamicBrokersReader.java ---
@@ -149,4 +158,19 @@ private Broker getBrokerHost(byte[] contents) {
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/361#discussion_r22435341
--- Diff:
external/storm-kafka/src/jvm/storm/kafka/DynamicBrokersReader.java ---
@@ -41,6 +41,14 @@
private String _topic;
public
GitHub user curtisallen opened a pull request:
https://github.com/apache/storm/pull/361
STORM-603 Log errors when required kafka params are missing
https://issues.apache.org/jira/browse/STORM-603
I was upgrading our topologies to storm-0.9.3 this
13 matches
Mail list logo
