[GitHub] storm pull request: Storm 509. Make groups checking specific for S...

2014-09-30 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/273#discussion_r18237335 --- Diff: storm-core/src/jvm/backtype/storm/security/auth/authorizer/SimpleACLAuthorizer.java --- @@ -107,12 +108,18 @@ public boolean permit(ReqContext

[GitHub] storm pull request: STORM-386 nodejs multilang protocol implementa...

2014-09-30 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/177#issuecomment-57363269 @itaifrenkel and @anyatch both of you should be in README.markdown now. Thanks again for your contribution. --- If your project is set up for it, you can reply

[GitHub] storm pull request: [STORM-529] Send fail(tup) when BasicBolt proc...

2014-10-15 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/291#issuecomment-59222813 @itaifrenkel Sorry for the long silence. We have been battling a fire here with all hands on deck. I don't see much of a reason for this to wait on STORM-528. You made

[GitHub] storm pull request: [STORM-529] Send fail(tup) when BasicBolt proc...

2014-10-15 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/291#issuecomment-59226497 I am +1 for this change. The code looks to do as expected, and everything runs well. --- If your project is set up for it, you can reply to this email and have your

[GitHub] storm pull request: [STORM-533] Added in client and server IConnec...

2014-10-24 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/302 [STORM-533] Added in client and server IConnection metrics. You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm iconn

[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...

2014-11-06 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-61983313 Sorry it took me so long to respond. The changes look good to me, but I don't have a windows box so I cannot test the windows support, I am +1 on the changes so far

[GitHub] storm pull request: [STORM-533] Added in client and server IConnec...

2014-11-06 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/302#issuecomment-61989690 The metrics system is very generic, and not that complex. Essentially it sets up a timer that will periodically call getValueAndReset on an instance of IMetric

[GitHub] storm pull request: STORM-216: Added Authentication and Authorizat...

2014-11-12 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/121#issuecomment-62754593 I think we are at the point where we should merge the security branch into master. This is a little bit different from most patches/pull requests we have done, partly

[GitHub] storm pull request: STORM-216: Added Authentication and Authorizat...

2014-11-12 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/121#issuecomment-62782715 Like I said in the mailing list, I have been out for a few weeks. I didn't see 0.9.3 go out, but I thought that it must be very close by now. I'll send a separate mail

[GitHub] storm pull request: [STORM-533] Added in client and server IConnec...

2014-11-14 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/302#issuecomment-63134572 @clockfly is it OK to merge this code in? I am happy to file a new JIRA for rethinking how we do metrics in storm, if you think that would be helpful. --- If your

[GitHub] storm pull request: [STORM-557] Created docs directory and added i...

2014-11-17 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/314#issuecomment-63332256 I was curious if we wanted to keep a 5MB file in our github repo that everyone must download to get storm? As it stands now the git repo is about 11MB total. This could

[GitHub] storm pull request: [STORM-557] Created docs directory and added i...

2014-11-17 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/314#issuecomment-63337668 OK so the next question I have is, should we split the one big SVG file up along side rasterized PNG equivalents for use in the docs? --- If your project is set up

[GitHub] storm pull request: [STORM-557] Created docs directory and added i...

2014-11-17 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/314#issuecomment-63354593 OK I split them apart, cleaned up one of the SVG images to make it smaller, and added in some PNG versions, along with updating the security document to use one

[GitHub] storm pull request: [STORM-410] Add groups support to log-viewer

2014-11-18 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/317 [STORM-410] Add groups support to log-viewer You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-410 Alternatively

[GitHub] storm pull request: STORM-188: Allow user to specifiy full configu...

2014-11-18 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/120#discussion_r20513760 --- Diff: storm-core/src/jvm/backtype/storm/utils/Utils.java --- @@ -122,36 +124,64 @@ public static void sleep(long millis

[GitHub] storm pull request: STORM-188: Allow user to specifiy full configu...

2014-11-18 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/120#issuecomment-63492543 I just had one comment, to avoid YAML doing some potentially unsafe things, but it is not that critical, because placing a --config on the command line is going to use

[GitHub] storm pull request: STORM-550:fix bug get an error when use --conf...

2014-11-18 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/310#issuecomment-63493192 This is a duplicate of STORM-188 as @clockfly suggested. This patch has a regression, in that it does not search the classpath for the --config option. I don't expect

[GitHub] storm pull request: STORM-188: Allow user to specifiy full configu...

2014-11-18 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/120#discussion_r20514725 --- Diff: storm-core/src/jvm/backtype/storm/utils/Utils.java --- @@ -122,36 +124,64 @@ public static void sleep(long millis

[GitHub] storm pull request: STORM-552:add new config storm.messaging.netty...

2014-11-18 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/311#issuecomment-63495857 @ptgoetz I traced this down in the netty code to the second parameter to the call to bind in the socket. https://docs.oracle.com/javase/7/docs/api/java/net

[GitHub] storm pull request: STORM-430: Allow netty SASL to support encrypt...

2014-11-18 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/250#issuecomment-63524216 Not a problem it is great work, I would like to see it in. Also note that security has been merged into master so if you could update your pull request to master

[GitHub] storm pull request: [STORM-570] replace table sorter with data tab...

2014-11-25 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/328 [STORM-570] replace table sorter with data table. Also upraded several ... ...javascript libraries to latest versions to make integration simpler. I know this is kind of a lot. I upgraded

[GitHub] storm pull request: [STORM-570] replace table sorter with data tab...

2014-11-26 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/328#issuecomment-64690420 Thanks for the advice. I switched the style over to be compact for the tables. I also fixed an issue with they layout on fire fox where the table always filled more than

[GitHub] storm pull request: [STORM-572] add favicon

2014-11-26 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/326#issuecomment-64700524 +1 sweet! @Parth-Brahmbhatt we don't the apache icon. We can add apache elsewhere on the UI if we feel a need to later, but there are no branding requirements

[GitHub] storm pull request: STORM-567: Move Storm Documentation/Website fr...

2014-11-26 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/330#issuecomment-64707784 For the most part it looks great. It looks like we could use some license headers in some of the files. My only other comment would be to add

[GitHub] storm pull request: [STORM-575] Specify UI/Jetty interface to bind...

2014-11-26 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/329#issuecomment-64722744 This looks fine to me +1. My only concern is that this pull request is against 0.9.3-branch and not master. I will leave it up to @ptgoetz if he wants to have the 0.9.3

[GitHub] storm pull request: [STORM-573]Allow users to pass TEST-TIMEOUT-MS...

2014-11-28 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/327#issuecomment-64931214 @javrasya We typically will resolve the JIRA when it is merged into a release branch, a.k.a master. When the release goes out then we close the JIRA. Thanks

[GitHub] storm pull request: use boolean to replace some?, backtype.storm.u...

2014-11-28 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/285#issuecomment-64933603 I like the change, but supervisor_test.clj needs to be updated too (same chage as before). It would also be good if you could file a JIRA for this at http

[GitHub] storm pull request: Update cluster.xml

2014-12-05 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/337#issuecomment-6573 +1 this change itself looks good, but there are a number of issues with the metrics.log file. I filed STORM-584 to address them. --- If your project is set up

[GitHub] storm pull request: Initiali check-in for storm-eventhubs.

2014-12-05 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/336#issuecomment-65800546 @shanyu This is a ton of code. Do you have a JIRA for this? Do you have a design document of some sort describing how this works? --- If your project is set up

[GitHub] storm pull request: STORM-577:supervisor heartbeat and handle ev...

2014-12-05 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/332#issuecomment-65801126 The change looks fine to me +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

[GitHub] storm pull request: STORM-567: Move Storm Documentation/Website fr...

2014-12-05 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/330#issuecomment-65801181 @ptgoetz any update on 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 does not have

[GitHub] storm pull request: [STORM-557] Created docs directory and added i...

2014-12-05 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/314#issuecomment-65801449 With pull #330 now up, I will wait for it to go in, and then rebase my changes on top of it. --- If your project is set up for it, you can reply to this email and have

[GitHub] storm pull request: use boolean to replace some?, backtype.storm.u...

2014-12-11 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/285#issuecomment-66633105 @taojoe It really doesn't matter that much if it is an improvement or a bug. If you want to file it as a bug that would be fine. After filing the JIRA please add

[GitHub] storm pull request: [STORM-557] Created docs directory and added i...

2014-12-22 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/314#issuecomment-67888564 Enough changed with the docs update, that I opted to just add the images in. If we want to move some of the other markdown documents over we can, but for now I just left

[GitHub] storm pull request: [STORM-410] Add groups support to log-viewer

2014-12-22 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/317#issuecomment-67890661 I upmerged and updated SECURITY.md to reflect the appropriate changes. Please take a look again. --- If your project is set up for it, you can reply to this email

[GitHub] storm pull request: [STORM-570] replace table sorter with data tab...

2014-12-22 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/328#issuecomment-67892015 I just upmerged. --- 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: Storm-444: Moving AutoHDFS to storm-hdfs and A...

2014-12-22 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/344#issuecomment-67895275 It looks fine to me +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

[GitHub] storm pull request: STORM-600: Update version of jacoco plugin.

2014-12-22 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/357 STORM-600: Update version of jacoco plugin. You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-600 Alternatively you

[GitHub] storm pull request: STORM-601: Make jira-github-join ignore case.

2014-12-23 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/358 STORM-601: Make jira-github-join ignore case. You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-601 Alternatively

[GitHub] storm pull request: STORM-544 Fix outdated documents

2014-12-23 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/340#issuecomment-67962843 +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

[GitHub] storm pull request: Storm-456 cannot navigate to topology page whe...

2014-12-23 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/318#issuecomment-67976452 The issue appears to be that when we url decode the topology name the '+' is staying as a '+' if I replace it with a space ' ' in the URL it seems to work for me on both

[GitHub] storm pull request: STORM-552:add new config storm.messaging.netty...

2014-12-23 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/311#issuecomment-67976914 @caofangkun any update on 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 does

[GitHub] storm pull request: STORM-505: Fix debug string construction

2014-12-23 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/359#issuecomment-67979768 +1 this is a rebase of https://github.com/apache/storm/pull/266 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] storm pull request: STORM-248 STORM-322 proposal for fix

2014-12-23 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/353#issuecomment-67982494 +1 the changes look fine to me. Please upmerge. the storm python script has changed since you did the pull request and there are some minor merge conflicts that should

[GitHub] storm pull request: STORM-243,Record version and revision informat...

2014-12-23 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/294#discussion_r8158 --- Diff: storm-core/src/clj/backtype/storm/ui/core.clj --- @@ -52,15 +52,9 @@ (map #(.get_stats ^ExecutorSummary %)) (filter not-nil

[GitHub] storm pull request: STORM-243,Record version and revision informat...

2014-12-23 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/294#issuecomment-67985049 @caofangkun thank you for submitting this patch. I appreciate you trying to help. In the future it would be good though, to be sure that your patch complies

[GitHub] storm pull request: [STORM-442] multilang ShellBolt/ShellSpout die...

2014-12-23 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/305#issuecomment-67989780 I see this has not had that much traffic in quite a while. The code to me looks acceptable. I am not really happy with calling .available() twice, but I can live

[GitHub] storm pull request: [STORM-591] exclude logback.xml from jar

2014-12-23 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/352#issuecomment-67997538 This seems to be used by the tests, after this change see a lot of DEBUG and INFO logs from zookeeper. Instead of deleting it, can we move it to a directory that is only

[GitHub] storm pull request: [STORM-599] Use use nimbus's cached heartbeats...

2014-12-23 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/356#issuecomment-67997939 The changes look fine to me. My only concern would be around what happens if the cache has not been updated and is empty, but it looks like it will return an empty map

[GitHub] storm pull request: STORM-598:Newly submitted topologies do not sh...

2014-12-23 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/355#discussion_r22236169 --- Diff: storm-core/src/clj/backtype/storm/daemon/nimbus.clj --- @@ -1248,7 +1256,7 @@ (when-let [owner

[GitHub] storm pull request: STORM-598:Newly submitted topologies do not sh...

2014-12-23 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/355#issuecomment-68001550 Also I tried to reproduce the issue on master and I am seeing the second topology come up on the ui without any issues. Could you try to reproduce the error on master

[GitHub] storm pull request: STORM-581. Add rebalance params to Storm REST ...

2015-01-23 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/370#issuecomment-71254799 +1 the code looks good. It needs to be upmerged, but other then that it looks fine. --- If your project is set up for it, you can reply to this email and have your

[GitHub] storm pull request: STORM-635. logviewer returns 404 if storm_home...

2015-01-23 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/391#issuecomment-71253695 +1 looks good to me. --- 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: [STORM-629] Place Link to Source Code Reposito...

2015-01-23 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/386#issuecomment-71263897 +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

[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-01-23 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/365#issuecomment-71273044 +1 the changes still look good Are there any other committers that want to take a look at the code? I would feel more comfortable with more people looking

[GitHub] storm pull request: STORM-598:Newly submitted topologies do not sh...

2015-01-23 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/355#issuecomment-71263582 The code compiles now, but I still have not been able to reproduce the original cause of this problem. am I doing something wrong? ``` ./bin/storm dev-zookeeper

[GitHub] storm pull request: Storm 528

2015-02-02 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/389#issuecomment-72539951 There is a lot of code in here that I don't understand how it relates to STORM-528. Setting the code dir, is not really something that I think we want to expose, except

[GitHub] storm pull request: update worker.clj-delete missing-tasks check...

2015-02-02 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/288#issuecomment-72536306 @BuDongDong I merged this in, but because your repo was deleted the pull request is not automatically recognizing that it was pulled in. Could you check master and close

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24043926 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcBolt.java --- @@ -0,0 +1,81 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24044371 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/Column.java --- @@ -0,0 +1,87 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24015372 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/Column.java --- @@ -0,0 +1,87 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24014941 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcBolt.java --- @@ -0,0 +1,81 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/374#issuecomment-72685012 For the most part it looks OK. I am concerned about a few of the APIs seem confusing to me and could use some better javadocs. Also I am concerned with the performance

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24014635 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcBolt.java --- @@ -0,0 +1,81 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24014770 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcBolt.java --- @@ -0,0 +1,91 @@ +/** + * Licensed to the Apache Software

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24015210 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/bolt/JdbcLookupBolt.java --- @@ -0,0 +1,86 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: [STORM-570] replace table sorter with data tab...

2015-02-03 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/328#issuecomment-72688638 I rebased it so it would merge cleanly. @kishorvpatil if you could take a look again that would be great. --- If your project is set up for it, you can reply

[GitHub] storm pull request: Storm-616 : Storm-jdbc connector.

2015-02-03 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24016216 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/JdbcClient.java --- @@ -0,0 +1,213 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: STORM-534:Store Nimbus Server Information in z...

2015-02-03 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/394#issuecomment-72693402 @caofangkun If you want to reopen this and just add in the nimbus version information I would support that. --- If your project is set up for it, you can reply

[GitHub] storm pull request: STORM-534:Store Nimbus Server Information in z...

2015-02-03 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/394#issuecomment-72693203 Sorry this took me so long to respond to. I am kind of swamped :). I do like the idea of being able to access the version numbers for nimbus, but also

[GitHub] storm pull request: STORM-350: LMAX Disruptor 3.2.1

2015-02-03 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/134#issuecomment-72693694 @darionyaphet, if 3.3.0 is out with a different wait strategy it would at least be worth doing a comparison. Do you have the time to do that? --- If your project

[GitHub] storm pull request: STORM-608. Storm UI CSRF escape characters not...

2015-02-03 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/371#issuecomment-72717689 +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

[GitHub] storm pull request: [STORM-400] Thrift upgrade to thrift-0.9.2

2015-02-06 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/410#issuecomment-73267260 The changes look good for the most part, but I am seeing a hang that comes up rather frequently. ``` main #1 prio=5 os_prio=31 tid=0x7fc339801000 nid

[GitHub] storm pull request: STORM-354: Testing: allow users to pass TEST-T...

2015-01-20 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/122#issuecomment-70688283 https://github.com/apache/storm/blob/master/storm-core/src/clj/backtype/storm/testing.clj#L190-192 Shows that the environment variable STORM_TEST_TIMEOUT_MS, can

[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-01-20 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/365#discussion_r23232733 --- Diff: external/storm-redis/src/main/java/org/apache/storm/redis/trident/state/RedisClusterMapState.java --- @@ -0,0 +1,295 @@ +/** + * Licensed

[GitHub] storm pull request: [STORM-609] Add storm-redis to storm external

2015-01-20 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/365#discussion_r23232475 --- Diff: external/storm-redis/pom.xml --- @@ -0,0 +1,108 @@ +?xml version=1.0 encoding=UTF-8? +!-- + Licensed to the Apache Software Foundation

[GitHub] storm pull request: STORM-626: Add script to print out the merge c...

2015-01-14 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/380 STORM-626: Add script to print out the merge command for a given pull request. You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2

[GitHub] storm pull request: [STORM-630] Support for Clojure 1.6.0

2015-01-22 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/390#issuecomment-71053295 +1 the code looks fine to me. --- 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: Storm-616 : Storm-jdbc connector.

2015-02-10 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/374#discussion_r24434272 --- Diff: external/storm-jdbc/src/main/java/org/apache/storm/jdbc/common/JdbcClient.java --- @@ -0,0 +1,213 @@ +/** + * Licensed to the Apache

[GitHub] storm pull request: STORM-329: fix cascading Storm failure by impr...

2015-02-17 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/429#discussion_r24824540 --- Diff: storm-core/src/jvm/backtype/storm/messaging/netty/Client.java --- @@ -42,344 +42,577 @@ import org.slf4j.LoggerFactory; import

[GitHub] storm pull request: STORM-329: fix cascading Storm failure by impr...

2015-02-17 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/429#issuecomment-74702976 I don't it could just be my mac acting funny. I'll dig into it, but don't block the pull request on me. I saw similar things on a different pull request that corrected

[GitHub] storm pull request: STORM-329: fix cascading Storm failure by impr...

2015-02-17 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/429#issuecomment-74696132 I am +1 for the code as is. Perhaps a separate JIRA for the reconnection attempts would be best. The issue has been in for quite a while now, and this code

[GitHub] storm pull request: STORM-329: fix cascading Storm failure by impr...

2015-02-17 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/429#issuecomment-74707726 OK It is something odd with my mac. It looks like the difference is wired vs wireless networking :), or possibly even switching between the two. --- If your project

[GitHub] storm pull request: Storm-456 cannot navigate to topology page whe...

2015-02-17 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/318#issuecomment-74683011 We put in some changes so the storm build now uses a plugin that is also built as part of the build. This makes it so you cannot run ```mvn compile``` on its own any

[GitHub] storm pull request: STORM-637: Integrate PartialKeyGrouping into s...

2015-01-30 Thread revans2
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/404 STORM-637: Integrate PartialKeyGrouping into storm API This updates the documentation and provides APIs similar to fieldsGrouping so that it is fairly simple to use. You can merge this pull request

[GitHub] storm pull request: [STORM-636] Faster, optional retrieval of last...

2015-01-28 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/392#issuecomment-71864014 I agree that ZK is not the ideal place to storm most things, but it is by far the most convenient. As such unless it is shown that it is causing a significant load on ZK

[GitHub] storm pull request: [STORM-636] Faster, optional retrieval of last...

2015-01-28 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/392#issuecomment-71864756 The code looks fine to me +1. Although I want to wait for @Parth-Brahmbhatt to finish his review before merging anything in. --- If your project is set up for it, you

[GitHub] storm pull request: STORM-243,Record version and revision informat...

2015-01-09 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/294#discussion_r22720543 --- Diff: storm-core/src/jvm/backtype/storm/utils/VersionInfo.java --- @@ -0,0 +1,58 @@ +package backtype.storm.utils; + +public class VersionInfo

[GitHub] storm pull request: STORM-243,Record version and revision informat...

2015-01-09 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/294#discussion_r22720414 --- Diff: storm-core/src/jvm/backtype/storm/package-info.java --- @@ -0,0 +1,7 @@ +/* --- End diff -- This file is also a build artifact

[GitHub] storm pull request: STORM-243,Record version and revision informat...

2015-01-09 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/294#discussion_r22720781 --- Diff: storm-core/src/saveVersion.sh --- @@ -0,0 +1,42 @@ +#this file is used to generate the package-info.java class that +# records the version

[GitHub] storm pull request: STORM-243,Record version and revision informat...

2015-01-09 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/294#issuecomment-69348667 @caofangkun Overall it looks really good, thanks for doing this. I have a few comments, mostly to clean things up a bit, or some follow up work. --- If your project

[GitHub] storm pull request: STORM-243,Record version and revision informat...

2015-01-09 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/294#discussion_r22720641 --- Diff: storm-core/src/saveVersion.sh --- @@ -0,0 +1,42 @@ +#this file is used to generate the package-info.java class that +# records the version

[GitHub] storm pull request: STORM-243,Record version and revision informat...

2015-01-09 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/294#discussion_r22721124 --- Diff: bin/storm --- @@ -88,12 +88,12 @@ def get_config_opts(): global CONFIG_OPTS return -Dstorm.options= + ','.join(map(quote_plus

[GitHub] storm pull request: STORM-243,Record version and revision informat...

2015-01-09 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/294#discussion_r22721149 --- Diff: storm-dist/binary/src/main/assembly/binary.xml --- @@ -131,13 +131,6 @@ /file !-- TODO this should be a generated file from

[GitHub] storm pull request: STORM-430: Allow netty SASL to support encrypt...

2015-01-09 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/250#issuecomment-69367006 @RaghavendraNandagopal any update on this? Don't want to lose this great work. If you are too busy I can try and upmerge it myself. --- If your project is set up

[GitHub] storm pull request: Update get-task-object function, change the ...

2015-01-09 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/312#issuecomment-69380814 @BuDongDong the changes you made look good to me. Would it be possible to get you to file a JIRA for them? This is mostly for tracking to make it simpler to see what

[GitHub] storm pull request: STORM-512 KafkaBolt doesn't handle ticks prope...

2015-01-09 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/275#issuecomment-69375242 The changes look find to me I am +1 for merging this in. @nathanmarz I would like to get your opinion on this before merging it in, because you had the original

[GitHub] storm pull request: STORM-248 STORM-322 are fixed and upmerged

2015-01-06 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/373#issuecomment-68935435 One minor comment, but other then that it looks good. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] storm pull request: STORM-248 STORM-322 are fixed and upmerged

2015-01-06 Thread revans2
Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/373#discussion_r22552440 --- Diff: bin/storm --- @@ -74,7 +74,8 @@ if (not os.path.isfile(os.path.join(USER_CONF_DIR, storm.yaml))): STORM_RELEASE_DIR = os.path.join(STORM_DIR

[GitHub] storm pull request: [STORM-442] multilang ShellBolt/ShellSpout die...

2015-01-06 Thread revans2
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/305#issuecomment-68936183 No other comments in the past two weeks so I am going to check this in. --- If your project is set up for it, you can reply to this email and have your reply appear

  1   2   3   4   5   6   7   8   9   10   >