[GitHub] storm issue #2237: [STORM-2653] Pacemaker code improvement

2017-07-24 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2237 @Ethanlm Great, now we need to wait for the full 24 hours to be up in case others want to respond to the pull request. --- If your project is set up for it, you can reply to this email and have

[GitHub] storm issue #2204: STORM-1280 port backtype.storm.daemon.logviewer to java

2017-07-24 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2204 The change looks good to me and I am +1. Will wait for @Ethanlm to work through his issues first though. --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] storm issue #2228: STORM-2646 NimbusClient Class cast exception when nimbus ...

2017-07-21 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2228 @ecararus could you please take a look at https://github.com/revans2/incubator-storm/commit/0978338c7dd7ec23b035abb5f70ce088064bf60b A lot of it you can ignore, but I added in a test on top

[GitHub] storm issue #2208: [STORM-2627] The annotation of storm.zookeeper.topology.a...

2017-07-21 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2208 @liu-zhaokun @Ethanlm is not a committer yet but I will try to pull it 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

[GitHub] storm issue #2204: STORM-1280 port backtype.storm.daemon.logviewer to java

2017-07-21 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2204 @HeartSaVioR I have not tested on a secure cluster at all. It was all on a single node cluster. I verified that as many links as I could worked correctly. I verified I could switch

[GitHub] storm issue #2204: STORM-1280 port backtype.storm.daemon.logviewer to java

2017-07-20 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2204 I did a bunch of testing and I am +1 on merging this in. There is still the issue with double escaping the contents returned for a file, but I think we can fix that after if we want. --- If your

[GitHub] storm issue #2204: STORM-1280 port backtype.storm.daemon.logviewer to java

2017-07-20 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2204 Overall it looks good. At least as an accurate translation of the clojure code. I want to play around with it some more, and also try to understand what is happening with some of the escaped

[GitHub] storm pull request #2204: STORM-1280 port backtype.storm.daemon.logviewer to...

2017-07-20 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2204#discussion_r128599677 --- Diff: storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/webapp/LogviewerResource.java --- @@ -0,0 +1,268 @@ +/* + * Licensed

[GitHub] storm pull request #2204: STORM-1280 port backtype.storm.daemon.logviewer to...

2017-07-20 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2204#discussion_r128599601 --- Diff: storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/LogviewerServer.java --- @@ -0,0 +1,182 @@ +/* + * Licensed to the Apache

[GitHub] storm issue #2201: STORM-2621: add tuple_population metric

2017-07-20 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2201 I rebased it and amended the commit message. @HeartSaVioR if you want to take a look again I would appreciate it, but the rebase was automatic. --- If your project is set up for it, you can reply

[GitHub] storm issue #2201: STORM-2557: add tuple_population metric

2017-07-20 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2201 @HeartSaVioR Sorry I have been out on vacation. Yes you are correct I copies and pasted the wrong thing. This is a fix for an issue caused by STORM-2557. I will update the pull request

[GitHub] storm issue #2228: STORM-2646 NimbusClient Class cast exception when nimbus ...

2017-07-20 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2228 The patch right now looks fine to me, but I don't think it fixes your issue with flux. Looking at the flux code inside FluxParser.java the `${NAME}` substitution is literately a macro substitution

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-07-20 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r128585169 --- Diff: storm-core/src/jvm/org/apache/storm/metrics2/DisruptorMetrics.java --- @@ -0,0 +1,93 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-07-20 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r128590434 --- Diff: storm-core/src/jvm/org/apache/storm/metrics2/StormMetricRegistry.java --- @@ -0,0 +1,133 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-07-20 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r128583930 --- Diff: examples/storm-starter/src/jvm/org/apache/storm/starter/ReliableWordCount.java --- @@ -0,0 +1,121 @@ +package org.apache.storm.starter

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-07-20 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r128582400 --- Diff: conf/defaults.yaml --- @@ -293,3 +293,28 @@ storm.daemon.metrics.reporter.plugins: # configuration of cluster metrics consumer

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-07-20 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r128585096 --- Diff: storm-core/src/jvm/org/apache/storm/Config.java --- @@ -139,6 +139,9 @@ @isString public static final String

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-07-20 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r128587178 --- Diff: storm-core/src/jvm/org/apache/storm/metrics2/StormMetricRegistry.java --- @@ -0,0 +1,133 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-07-20 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r128584145 --- Diff: storm-core/pom.xml --- @@ -700,10 +707,10 @@ org.eclipse.jetty

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-07-20 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r128583348 --- Diff: examples/storm-starter/src/jvm/org/apache/storm/starter/ReliableWordCount.java --- @@ -0,0 +1,121 @@ +package org.apache.storm.starter

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-07-20 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r128587605 --- Diff: storm-core/src/jvm/org/apache/storm/metrics2/reporters/GangliaStormReporter.java --- @@ -0,0 +1,133 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-07-20 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r128583613 --- Diff: examples/storm-starter/src/jvm/org/apache/storm/starter/ReliableWordCount.java --- @@ -0,0 +1,121 @@ +package org.apache.storm.starter

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-07-20 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r128582631 --- Diff: examples/storm-starter/src/jvm/org/apache/storm/starter/ReliableWordCount.java --- @@ -0,0 +1,121 @@ +package org.apache.storm.starter

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-07-20 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r128584007 --- Diff: external/storm-autocreds/pom.xml --- @@ -15,9 +15,7 @@ See the License for the specific language governing permissions and limitations

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-07-20 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r128591566 --- Diff: storm-core/src/jvm/org/apache/storm/task/TopologyContext.java --- @@ -386,4 +388,28 @@ public ReducedMetric registerMetric(String name, IReducer

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-07-20 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r128582529 --- Diff: conf/defaults.yaml --- @@ -293,3 +293,28 @@ storm.daemon.metrics.reporter.plugins: # configuration of cluster metrics consumer

[GitHub] storm pull request #2203: STORM-2153: New Metrics Reporting API

2017-07-20 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2203#discussion_r128591352 --- Diff: storm-core/src/jvm/org/apache/storm/task/TopologyContext.java --- @@ -386,4 +388,28 @@ public ReducedMetric registerMetric(String name, IReducer

[GitHub] storm pull request #2204: STORM-1280 port backtype.storm.daemon.logviewer to...

2017-07-19 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2204#discussion_r128356199 --- Diff: storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/LogviewerServer.java --- @@ -0,0 +1,182 @@ +/* + * Licensed to the Apache

[GitHub] storm pull request #2204: STORM-1280 port backtype.storm.daemon.logviewer to...

2017-07-19 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2204#discussion_r128364464 --- Diff: storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/LogviewerServer.java --- @@ -0,0 +1,182 @@ +/* + * Licensed to the Apache

[GitHub] storm pull request #2204: STORM-1280 port backtype.storm.daemon.logviewer to...

2017-07-19 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2204#discussion_r128366130 --- Diff: storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/webapp/LogviewerResource.java --- @@ -0,0 +1,268 @@ +/* + * Licensed

[GitHub] storm pull request #2204: STORM-1280 port backtype.storm.daemon.logviewer to...

2017-07-19 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2204#discussion_r128366771 --- Diff: storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/webapp/LogviewerResource.java --- @@ -0,0 +1,268 @@ +/* + * Licensed

[GitHub] storm pull request #2204: STORM-1280 port backtype.storm.daemon.logviewer to...

2017-07-19 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2204#discussion_r128366709 --- Diff: storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/webapp/LogviewerResource.java --- @@ -0,0 +1,268 @@ +/* + * Licensed

[GitHub] storm pull request #2204: STORM-1280 port backtype.storm.daemon.logviewer to...

2017-07-19 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2204#discussion_r128367236 --- Diff: storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/webapp/LogviewerResource.java --- @@ -0,0 +1,268 @@ +/* + * Licensed

[GitHub] storm pull request #2204: STORM-1280 port backtype.storm.daemon.logviewer to...

2017-07-19 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2204#discussion_r128368272 --- Diff: storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/handler/LogviewerLogPageHandler.java --- @@ -0,0 +1,467 @@ +/* + * Licensed

[GitHub] storm pull request #2204: STORM-1280 port backtype.storm.daemon.logviewer to...

2017-07-19 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2204#discussion_r128356585 --- Diff: storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/LogviewerServer.java --- @@ -0,0 +1,182 @@ +/* + * Licensed to the Apache

[GitHub] storm pull request #2204: STORM-1280 port backtype.storm.daemon.logviewer to...

2017-07-19 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2204#discussion_r128353794 --- Diff: storm-core/src/clj/org/apache/storm/ui/core.clj --- @@ -134,12 +134,12 @@ (defn logviewer-link [host fname secure

[GitHub] storm pull request #2204: STORM-1280 port backtype.storm.daemon.logviewer to...

2017-07-19 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2204#discussion_r128366249 --- Diff: storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/webapp/LogviewerResource.java --- @@ -0,0 +1,268 @@ +/* + * Licensed

[GitHub] storm issue #2204: STORM-1280 port backtype.storm.daemon.logviewer to java

2017-07-19 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2204 I was playing around with it and I am seeing `` in the output log. i.e. ``` 2017-07-19 15:00:49.413 o.a.s.d.s.Slot SLOT_6701 [INFO] STATE EMPTY msInState: 38110

[GitHub] storm pull request #2208: [STORM-2627] The annotation of storm.zookeeper.top...

2017-07-19 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2208#discussion_r128345781 --- Diff: storm-client/src/jvm/org/apache/storm/Config.java --- @@ -967,12 +967,6 @@ public static final String STORM_ZOOKEEPER_SUPERACL

[GitHub] storm issue #2208: [STORM-2627] The annotation of storm.zookeeper.topology.a...

2017-07-19 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2208 Sorry I have been out of town or I would have joined the conversation sooner. Yes @HeartSaVioR is correct. The code is really confusing and overly complicated. I comes from when we were

[GitHub] storm issue #2216: Update storm.py to be python3 compatible

2017-07-19 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2216 ``` diff --git a/storm-multilang/python/src/main/resources/resources/storm.py b/storm-multilang/python/src/main/resources/resources/storm.py index 9106390b4..224ed120b 100755 --- a/storm

[GitHub] storm issue #2216: Update storm.py to be python3 compatible

2017-07-19 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2216 @rukaya I am +1 on the change, but we need to have a the change go into master before it goes into any of the 1.x branches. Do you have a pull request for master that I missed

[GitHub] storm issue #2225: STORM-2641: Make storm.py print output from subprocess on...

2017-07-19 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2225 The test failure looks unrelated. --- 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 #2228: fix class cast exception when nimbus seeds is not ...

2017-07-19 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2228#discussion_r128316430 --- Diff: storm-client/src/jvm/org/apache/storm/utils/NimbusClient.java --- @@ -109,12 +109,13 @@ public static NimbusClient getConfiguredClientAs(Map

[GitHub] storm issue #2201: STORM-2557: add tuple_population metric

2017-07-10 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2201 I ran some performance tests and the extra overhead is minimal. --- 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 #2200: STORM-2616: Documentation for built in metrics

2017-07-10 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2200 STORM-2616: Documentation for built in metrics I am happy to pull this back into 1.x branches too. I am not sure exactly how the metrics might have changed though, so we might want to be careful

[GitHub] storm issue #2164: [STORM-2557]: A bug in DisruptorQueue causing severe unde...

2017-07-10 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2164 I just realized that because the `sojourn_time_ms` is calculated based off of the request rate, that this updated, and the population (not updated and still based off of slots and not tuples

[GitHub] storm issue #2198: STORM-2620: Updated docs for JDK needed (1.x)

2017-07-10 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2198 This is the 1.x pull request for #2197 --- 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 #2198: STORM-2620: Updated docs for JDK needed

2017-07-10 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2198 STORM-2620: Updated docs for JDK needed This should be able to be pulled into all of the 1.x release branches without any issues. You can merge this pull request into a Git repository by running

[GitHub] storm pull request #2197: STORM-2620: Updated docs for JDK needed

2017-07-10 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2197 STORM-2620: Updated docs for JDK needed I plan to do a separate pull request to the 1.x branch(s) too. You can merge this pull request into a Git repository by running: $ git pull https

[GitHub] storm issue #2184: STORM-2610: Fixed the metrics

2017-07-07 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2184 @HeartSaVioR I filed STORM-2616 to document all of the built in metrics, and I'll try to get a pull request for that up some time soon. For now I will leave this as a 2.x change

[GitHub] storm issue #2032: [STORM-2093] Fix permissions in multi-tenant, secure mode

2017-07-05 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2032 @HeartSaVioR I'm not sure if @ppoulosk is going to be able to finish this, so @Ethanlm has submitted #2187 to take this over. --- If your project is set up for it, you can reply to this email

[GitHub] storm pull request #2184: STORM-2610: Fixed the metrics

2017-07-05 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2184#discussion_r125715402 --- Diff: storm-client/src/jvm/org/apache/storm/daemon/metrics/SpoutThrottlingMetrics.java --- @@ -26,32 +27,20 @@ private final CountMetric

[GitHub] storm pull request #2184: STORM-2610: Fixed the metrics

2017-07-05 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2184#discussion_r125715307 --- Diff: storm-client/src/jvm/org/apache/storm/utils/NimbusClient.java --- @@ -96,6 +96,10 @@ public static NimbusClient getConfiguredClientAs(Map<Str

[GitHub] storm issue #2183: STORM-2609: Simple command line DRPC Client

2017-07-05 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2183 @HeartSaVioR and @vesense I just updated the docs --- 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 #2184: STORM-2610: Fixed the metrics

2017-06-30 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2184 STORM-2610: Fixed the metrics I still need to add these to the docs, but I wanted to capture my progress before the long weekend. You can merge this pull request into a Git repository by running

[GitHub] storm pull request #2183: STORM-2609: Simple command line DRPC Client

2017-06-30 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2183 STORM-2609: Simple command line DRPC Client You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2609 Alternatively

[GitHub] storm pull request #2180: [STORM-2602] storm.zookeeper.topology.auth.payload...

2017-06-29 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2180#discussion_r124883491 --- Diff: storm-client/src/jvm/org/apache/storm/StormSubmitter.java --- @@ -78,21 +78,18 @@ public static boolean validateZKDigestPayload(String payload

[GitHub] storm pull request #2180: [STORM-2602] storm.zookeeper.topology.auth.payload...

2017-06-29 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2180#discussion_r124883536 --- Diff: storm-client/src/jvm/org/apache/storm/StormSubmitter.java --- @@ -78,21 +78,18 @@ public static boolean validateZKDigestPayload(String payload

[GitHub] storm issue #2180: [STORM-2602] storm.zookeeper.topology.auth.payload doesn'...

2017-06-29 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2180 On a related note this change looks to have broken ``` org.apache.storm.submitter-test / testname: test-md5-digest-secret-generation ``` So please take a look at why

[GitHub] storm issue #2180: [STORM-2602] storm.zookeeper.topology.auth.payload doesn'...

2017-06-29 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2180 @liu-zhaokun I read through the code and I would like to confirm how you are setting the payload. It looks like you are setting the payload in the storm.yaml file, and not passing

[GitHub] storm issue #2173: STORM-2597: Don't parse passed in class paths

2017-06-29 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2173 Sounds good. I'll merge it in, and see what, if anything I need to do to pull this into older version of storm too. --- If your project is set up for it, you can reply to this email and have your

[GitHub] storm issue #2142: MINOR: Fix pacemaker_state_factory.clj not compile proble...

2017-06-29 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2142 @HeartSaVioR yes. --- 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 #2173: STORM-2597: Don't parse passed in class paths

2017-06-26 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2173 @harshach yes this will cause issues with anyone currently setting a classpath to a directory that has jars in it. But it also make it behave like a java classpath currently does everywhere else

[GitHub] storm pull request #2173: STORM-2597: Don't parse passed in class paths

2017-06-23 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2173 STORM-2597: Don't parse passed in class paths You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-2597 Alternatively

[GitHub] storm issue #2149: STORM-2503: Fix lgtm.com alerts on equality and compariso...

2017-06-23 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2149 Yes I just put up a pull request that makes the ordering of the executors consistent. I don't know what changed between JDK7 and JDK 8 that would cause the order of the executors to be different

[GitHub] storm issue #2149: STORM-2503: Fix lgtm.com alerts on equality and compariso...

2017-06-23 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2149 OK It looks like it is a difference between java7 and java8, and the default ordering of something. I'll try to see if I can make it less ambiguous. --- If your project is set up for it, you can

[GitHub] storm issue #2149: STORM-2503: Fix lgtm.com alerts on equality and compariso...

2017-06-23 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2149 @HeartSaVioR both of those are valid schedulings and I am not totally sure why one would be picked in one release vs another. I'll try to reproduce the issue and see what I can come up

[GitHub] storm issue #2083: STORM-2421: support lists of childopts in DaemonConfig.

2017-06-22 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2083 @mal that is not an issue right now. There are a lot of changes to the code, but most of that is creating some common helper methods and refactoring. The only configs really impacted

[GitHub] storm issue #2161: STORM-2556 Break down AutoCreds implementations into two ...

2017-06-21 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2161 @HeartSaVioR I don't think more generalization is a benefit. My comment was mostly around making it hadoop specific or truly making it generic. --- If your project is set up for it, you can reply

[GitHub] storm issue #2164: [STORM-2557]: A bug in DisruptorQueue causing severe unde...

2017-06-19 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2164 The change as is looks OK to me. My biggest problem with it is that there is now tight coupling between the Disruptor queue and storm itself, although looking at the code that was true before

[GitHub] storm issue #2161: STORM-2556 Break down AutoCreds implementations into two ...

2017-06-15 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2161 I am fine with splitting things up, but I am not super excited about how we setup the Abstract* classes. Perhaps it is just how we are naming them, because there are a lot of things in them

[GitHub] storm issue #2113: STORM-2497: Let Supervisor enforce memory and add in supp...

2017-06-07 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2113 @srdo I think I got the rest of them @kishorvpatil could you take a look at let me know if the rework is good for you? --- If your project is set up for it, you can reply to this email

[GitHub] storm pull request #2113: STORM-2497: Let Supervisor enforce memory and add ...

2017-06-07 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2113#discussion_r120688356 --- Diff: storm-server/src/test/java/org/apache/storm/daemon/nimbus/NimbusTest.java --- @@ -24,8 +24,10 @@ import

[GitHub] storm pull request #2113: STORM-2497: Let Supervisor enforce memory and add ...

2017-06-07 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2113#discussion_r120687251 --- Diff: storm-client/src/jvm/org/apache/storm/scheduler/SchedulerAssignment.java --- @@ -21,41 +21,52 @@ import java.util.Map; import

[GitHub] storm issue #2113: STORM-2497: Let Supervisor enforce memory and add in supp...

2017-06-07 Thread revans2
Github user revans2 commented on the issue: https://github.com/apache/storm/pull/2113 I think I addressed all of the 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 #2113: STORM-2497: Let Supervisor enforce memory and add ...

2017-06-07 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2113#discussion_r120671482 --- Diff: storm-server/src/main/java/org/apache/storm/localizer/AsyncLocalizer.java --- @@ -144,8 +148,19 @@ public Void call() throws Exception

[GitHub] storm pull request #2113: STORM-2497: Let Supervisor enforce memory and add ...

2017-06-07 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2113#discussion_r120658098 --- Diff: storm-server/src/main/java/org/apache/storm/localizer/AsyncLocalizer.java --- @@ -144,8 +148,19 @@ public Void call() throws Exception

[GitHub] storm pull request #2113: STORM-2497: Let Supervisor enforce memory and add ...

2017-06-07 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2113#discussion_r120656010 --- Diff: storm-server/src/main/java/org/apache/storm/daemon/supervisor/Container.java --- @@ -528,28 +577,108 @@ public void cleanUpForRestart() throws

[GitHub] storm pull request #2113: STORM-2497: Let Supervisor enforce memory and add ...

2017-06-07 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2113#discussion_r120654636 --- Diff: storm-server/src/main/java/org/apache/storm/container/cgroup/CgroupManager.java --- @@ -151,22 +161,40 @@ public void reserveResourcesForWorker

[GitHub] storm pull request #2113: STORM-2497: Let Supervisor enforce memory and add ...

2017-06-07 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2113#discussion_r120648513 --- Diff: storm-client/src/jvm/org/apache/storm/trident/operation/DefaultResourceDeclarer.java --- @@ -29,8 +33,8 @@ */ public class

[GitHub] storm pull request #2113: STORM-2497: Let Supervisor enforce memory and add ...

2017-06-07 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2113#discussion_r120647251 --- Diff: storm-client/src/jvm/org/apache/storm/topology/BaseConfigurationDeclarer.java --- @@ -52,18 +52,20 @@ public T setNumTasks(Number val

[GitHub] storm pull request #2113: STORM-2497: Let Supervisor enforce memory and add ...

2017-06-07 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2113#discussion_r120646636 --- Diff: storm-client/src/jvm/org/apache/storm/topology/BaseConfigurationDeclarer.java --- @@ -52,18 +52,20 @@ public T setNumTasks(Number val

[GitHub] storm pull request #2113: STORM-2497: Let Supervisor enforce memory and add ...

2017-06-07 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2113#discussion_r120645730 --- Diff: storm-client/src/jvm/org/apache/storm/scheduler/SchedulerAssignmentImpl.java --- @@ -141,4 +222,42 @@ public String getTopologyId

[GitHub] storm pull request #2113: STORM-2497: Let Supervisor enforce memory and add ...

2017-06-07 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2113#discussion_r120644839 --- Diff: storm-client/src/jvm/org/apache/storm/scheduler/SchedulerAssignmentImpl.java --- @@ -19,36 +19,82 @@ import java.util.ArrayList

[GitHub] storm pull request #2148: Let Topology query if blobs have changed

2017-06-05 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2148#discussion_r120174827 --- Diff: storm-server/src/main/java/org/apache/storm/localizer/Localizer.java --- @@ -226,9 +218,8 @@ public boolean accept(File dir, String name

[GitHub] storm pull request #2148: Let Topology query if blobs have changed

2017-06-05 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2148#discussion_r120174018 --- Diff: storm-client/src/jvm/org/apache/storm/daemon/worker/Worker.java --- @@ -246,7 +248,19 @@ public void start() throws Exception

[GitHub] storm pull request #2148: Let Topology query if blobs have changed

2017-06-05 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2148#discussion_r120132248 --- Diff: storm-server/src/main/java/org/apache/storm/localizer/Localizer.java --- @@ -41,10 +42,7 @@ import java.io.PrintWriter; import

[GitHub] storm pull request #2148: Let Topology query if blobs have changed

2017-06-05 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2148#discussion_r120134652 --- Diff: storm-client/src/jvm/org/apache/storm/daemon/worker/Worker.java --- @@ -299,6 +313,33 @@ public void doExecutorHeartbeats

[GitHub] storm pull request #2148: Let Topology query if blobs have changed

2017-06-05 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2148#discussion_r120130165 --- Diff: storm-client/src/jvm/org/apache/storm/task/TopologyContext.java --- @@ -57,13 +57,14 @@ private Map<String, Object> _execut

[GitHub] storm pull request #2148: Let Topology query if blobs have changed

2017-06-05 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2148#discussion_r120132118 --- Diff: storm-client/src/jvm/org/apache/storm/task/TopologyContext.java --- @@ -57,13 +57,14 @@ private Map<String, Object> _execut

[GitHub] storm pull request #2148: Let Topology query if blobs have changed

2017-06-05 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2148#discussion_r120130996 --- Diff: storm-client/src/jvm/org/apache/storm/Constants.java --- @@ -56,5 +56,7 @@ public static final String STORM_ACTIVE_ATOM = "storm-a

[GitHub] storm pull request #2148: Let Topology query if blobs have changed

2017-06-05 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2148#discussion_r120135498 --- Diff: storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java --- @@ -2369,7 +2369,7 @@ public void launchServer() throws Exception

[GitHub] storm pull request #2148: Let Topology query if blobs have changed

2017-06-05 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2148#discussion_r120129840 --- Diff: storm-client/src/jvm/org/apache/storm/daemon/worker/Worker.java --- @@ -246,7 +248,19 @@ public void start() throws Exception

[GitHub] storm pull request #2148: Let Topology query if blobs have changed

2017-06-05 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2148#discussion_r120130253 --- Diff: storm-client/src/jvm/org/apache/storm/utils/ConfigUtils.java --- @@ -27,12 +28,11 @@ import java.io.File; import java.io.IOException

[GitHub] storm pull request #2148: Let Topology query if blobs have changed

2017-06-05 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2148#discussion_r120129279 --- Diff: storm-client/src/jvm/org/apache/storm/daemon/worker/Worker.java --- @@ -246,7 +248,19 @@ public void start() throws Exception

[GitHub] storm pull request #2148: Let Topology query if blobs have changed

2017-06-05 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2148#discussion_r120134821 --- Diff: storm-client/src/jvm/org/apache/storm/daemon/worker/Worker.java --- @@ -299,6 +313,33 @@ public void doExecutorHeartbeats

[GitHub] storm pull request #2148: Let Topology query if blobs have changed

2017-06-05 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2148#discussion_r120131569 --- Diff: storm-client/src/jvm/org/apache/storm/daemon/worker/Worker.java --- @@ -299,6 +313,33 @@ public void doExecutorHeartbeats

[GitHub] storm pull request #2148: Let Topology query if blobs have changed

2017-06-05 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2148#discussion_r120130332 --- Diff: storm-client/src/jvm/org/apache/storm/utils/ConfigUtils.java --- @@ -27,12 +28,11 @@ import java.io.File; import java.io.IOException

[GitHub] storm pull request #2148: Let Topology query if blobs have changed

2017-06-05 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2148#discussion_r120131449 --- Diff: storm-client/src/jvm/org/apache/storm/daemon/worker/Worker.java --- @@ -299,6 +313,33 @@ public void doExecutorHeartbeats

<    5   6   7   8   9   10   11   12   13   14   >