[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user harshach commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-74791899 @HeartSaVioR @dashengju I took care of it as part of this PR https://github.com/apache/storm/pull/434 . Thanks. --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-74376482 @dashengju @harshach Since there's no exec on Windows, we can check os, and use sub.call() if it's Windows, and use os.execvp() if it's not Windows. If you think it makes sense, I'll file new JIRA and post a relevant PR. Thanks! --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user dashengju commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-72795383 @harshach , ok, you or @HeartSaVioR should responsible for fix this bug, because I just know how to fix it in linux, but not in windows. thanks --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user harshach commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-72794546 @dashengju I second that ,just recently noticed it . We can file a follow-up jira for it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user dashengju commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-72793961 @harshach @HeartSaVioR @revans2 , this PR has produce a new bug in linux OS. In function exec_storm_classï¼line 181: -os.execvp(JAVA_CMD, all_args) # replaces the current process and -# never returns +# handling whitespaces in JAVA_CMD +sub.call(all_args) The origin code execute JAVA_CMD in exec style(replaces the current process and never returns), But the new code execute JAVA_CMD in sub process. Usually, nimbus/supervisor is running by daemon tools, so when nimbus/supervisor run in sub process, daemon tools can not stop the nimbus/supervisor process. --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user harshach commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-67860921 Thanks @HeartSaVioR merged PR. --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/280 --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user caofangkun commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-67813766 Should this be pluggable ? storm-master/bin/storm Python Script storm-master/external/storm-cli --- Proivide Bash Shell / Windows cmd Shell or any other launching script --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-67792351 @harshach Otherwise we can try to change storm.cmd to call bin\storm. I heard storm.cmd can register storm to Windows Service, so we could achieve it by modifying storm.cmd. @ChitturiPadma Yes, it will add prerequisite. It could be bothering for users, but it could save storm dev. to remove sth. that runs only Windows. As you can see, Storm committers doesn't use Windows. --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user ChitturiPadma commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-67764141 @HeartSavioR, if it works on distributed mode in windows cluster without any exceptions, you cab go ahead and merge it. But the users on windows should be given prerequisite to install Python. --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user harshach commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-67741881 follow-up feedback. In windows shebang line doesn't work so the users have to do python bin\storm nimbus etc.. . This can be solved by add .py extension to bin\storm . We can do this in a separate JIRA. @HeartSaVioR I'll do the merge. Thanks. --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-67735825 @harshach Thanks for reviewing! :) @ptgoetz @revans2 Since it would be tested from @ChitturiPadma and @harshach, and two +1 from committers, so we can merge it. :D --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user harshach commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-67712765 @HeartSaVioR sorry about taking so long to review this. Its been a challenge to get a windows box :). I tested this both on windows 8 and centos6.5 I tested all the commands in storm.py everything works as expected. I am +1 on merging this. Thanks. --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-65885963 Could anybody check/review 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 this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
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, but would like to see someone test this on windows too. --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user ptgoetz commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-61919968 @HeartSaVioR sorry for the delay. I will test and reply when I have the chance. I would encourage others to do the same 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 enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-61908678 @revans2 @ptgoetz Hello. Any updates on it? From some JIRA issues we already talked that it will be better to have 'unified' storm script. So when my patch works well, we reduce great maintenance cost. And it may can resolve STORM-322, too. :) --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-58017099 @ChitturiPadma Oh, that means PR can resolve STORM-487. Btw, I'm using apache-0.9.2-incubating. Actually I don't use Windows (I use OSX), but I'm just willing to contribute Storm Project and STORM-487 is labeled 'newbie'. --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user ChitturiPadma commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-58015873 I tested with apache-0.9.2-incubating version, copying the changes. It worked fine in pseduo distributed mode. Which version of storm have u been using... Earlier in storm-0.9.0.1 version the error of not able to delete the file is thrown from FileUtils.java in commons-io. --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user ChitturiPadma commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-57932225 When i worked on storm-0.9.0-rc2, the issue existed only with modified python script that works on windows. storm.cmd doesnt show have this issue in file deletion. Will recheck with latest storm-0.9.2 release and will get back to you. --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/280#issuecomment-57930437
@ChitturiPadma I've just test killing topology using Storm 0.9.2.
There're some warning but supervisor seems to not killed.
It seems to have an issue you mentioned (about file deletion) but workers
completely killed.
I can re-submit topology and it's running fine.
```
2014-10-05 17:48:50 b.s.d.supervisor [INFO] Shutting down
95111631-8f7f-46ed-a93a-8db1d01ae4e0:154eaf07-90ed-4f84-ad84-3b3b4eb51444
2014-10-05 17:48:53 b.s.d.supervisor [INFO] Shutting down and clearing
state for id 154eaf07-90ed-4f84-ad84-3b3b4eb51444. Current supervisor time:
1412498932. State: :disallowed, Heartbeat:
#backtype.storm.daemon.common.WorkerHeartbeat{:time-secs 1412498932, :storm-id
"production-topology-1-1412498729", :executors #{[2 2] [5 5] [8 8] [11 11] [14
14] [17 17] [20 20] [23 23] [26 26] [-1 -1]}, :port 6701}
2014-10-05 17:48:53 b.s.d.supervisor [INFO] Shutting down
95111631-8f7f-46ed-a93a-8db1d01ae4e0:154eaf07-90ed-4f84-ad84-3b3b4eb51444
2014-10-05 17:48:59 b.s.d.supervisor [INFO] Shut down
95111631-8f7f-46ed-a93a-8db1d01ae4e0:154eaf07-90ed-4f84-ad84-3b3b4eb51444
2014-10-05 17:48:59 b.s.d.supervisor [INFO] Shutting down
95111631-8f7f-46ed-a93a-8db1d01ae4e0:816a0fd6-7cdf-4312-a5ac-758f7f55ce46
2014-10-05 17:48:59 b.s.util [INFO] Error when trying to kill 4408. Process
is probably already dead.
2014-10-05 17:49:03 b.s.util [INFO] Error when trying to kill 4420. Process
is probably already dead.
2014-10-05 17:49:03 b.s.d.supervisor [INFO] Shut down
95111631-8f7f-46ed-a93a-8db1d01ae4e0:154eaf07-90ed-4f84-ad84-3b3b4eb51444
2014-10-05 17:49:03 b.s.d.supervisor [INFO] Shutting down and clearing
state for id 816a0fd6-7cdf-4312-a5ac-758f7f55ce46. Current supervisor time:
1412498932. State: :disallowed, Heartbeat:
#backtype.storm.daemon.common.WorkerHeartbeat{:time-secs 1412498932, :storm-id
"production-topology-1-1412498729", :executors #{[3 3] [6 6] [9 9] [12 12] [15
15] [18 18] [21 21] [24 24] [27 27] [-1 -1]}, :port 6702}
2014-10-05 17:49:03 b.s.d.supervisor [INFO] Shutting down
95111631-8f7f-46ed-a93a-8db1d01ae4e0:816a0fd6-7cdf-4312-a5ac-758f7f55ce46
2014-10-05 17:49:04 b.s.d.supervisor [WARN] Failed to cleanup worker
816a0fd6-7cdf-4312-a5ac-758f7f55ce46. Will retry later #
2014-10-05 17:49:04 b.s.d.supervisor [INFO] Shut down
95111631-8f7f-46ed-a93a-8db1d01ae4e0:816a0fd6-7cdf-4312-a5ac-758f7f55ce46
2014-10-05 17:49:04 b.s.d.supervisor [INFO] Shutting down and clearing
state for id c26b2e31-d006-4d4d-9e56-45ca9391018d. Current supervisor time:
1412498932. State: :disallowed, Heartbeat:
#backtype.storm.daemon.common.WorkerHeartbeat{:time-secs 1412498932, :storm-id
"production-topology-1-1412498729", :executors #{[4 4] [7 7] [10 10] [13 13]
[16 16] [19 19] [22 22] [25 25] [28 28] [-1 -1] [1 1]}, :port 6700}
2014-10-05 17:49:04 b.s.d.supervisor [INFO] Shutting down
95111631-8f7f-46ed-a93a-8db1d01ae4e0:c26b2e31-d006-4d4d-9e56-45ca9391018d
2014-10-05 17:49:04 b.s.d.supervisor [WARN] Failed to cleanup worker
816a0fd6-7cdf-4312-a5ac-758f7f55ce46. Will retry later #
2014-10-05 17:49:04 b.s.d.supervisor [INFO] Shut down
95111631-8f7f-46ed-a93a-8db1d01ae4e0:816a0fd6-7cdf-4312-a5ac-758f7f55ce46
2014-10-05 17:49:04 b.s.d.supervisor [INFO] Shutting down
95111631-8f7f-46ed-a93a-8db1d01ae4e0:c26b2e31-d006-4d4d-9e56-45ca9391018d
2014-10-05 17:49:04 b.s.util [INFO] Error when trying to kill 1536. Process
is probably already dead.
2014-10-05 17:49:06 b.s.util [INFO] Error when trying to kill 3408. Process
is probably already dead.
2014-10-05 17:49:06 b.s.util [INFO] Error when trying to kill 3464. Process
is probably already dead.
2014-10-05 17:49:06 b.s.d.supervisor [WARN] Failed to cleanup worker
c26b2e31-d006-4d4d-9e56-45ca9391018d. Will retry later #
2014-10-05 17:49:06 b.s.d.supervisor [INFO] Shut down
95111631-8f7f-46ed-a93a-8db1d01ae4e0:c26b2e31-d006-4d4d-9e56-45ca9391018d
2014-10-05 17:49:06 b.s.d.supervisor [WARN] Failed to cleanup worker
c26b2e31-d006-4d4d-9e56-45ca9391018d. Will retry later #
2014-10-05 17:49:06 b.s.d.supervisor [INFO] Shut down
95111631-8f7f-46ed-a93a-8db1d01ae4e0:c26b2e31-d006-4d4d-9e56-45ca9391018d
2014-10-05 17:49:06 b.s.d.supervisor [INFO] Removing code for storm id
production-topology-1-1412498729
2014-10-05 17:49:06 b.s.d.supervisor [INFO] Shutting down and clearing
state for id 816a0fd6-7cdf-4312-a5ac-758f7f55ce46. Current supervisor time:
1412498946. State: :not-started, Heartbeat: nil
2014-10-05 17:49:06 b.s.d.supervisor [INFO] Shutting down
95111631-8f7f-46ed-a93a-8db1d01ae4e0:816a0fd6-7cdf-4312-a5ac-758f7f55ce46
2014-10-05 17:49:06 b.s.d.supervisor [INFO] Shut down
95111631-8f7f-46ed-a93a-8db1d01ae4e0:816a0fd6-7cdf-4312-a5ac-758f7f55ce46
2014-10-05 17:49:06 b.s.d.supervisor [INFO] Shutting down and
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-57926268 @ChitturiPadma Thanks for sharing experience. I'll test on it. Btw, does storm.cmd runs normally with this scenario? --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user ChitturiPadma commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-57925935 I have tried this earlier. It runs the nimbus and supervisors without any issues and topology could be submitted without any issues. But when we try to kill the topology, supervisors would be terminated abruptly. This is happening because workers hold the topology jar file and internally clojure code first tries to remove the file and then kill the topology. Unfortunately this works in linux but windows doesnt allow to remove a file without closing the resources holding it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-57924831 @revans2 @ptgoetz Could you take a look, please? --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/280#issuecomment-57764337 In Windows, users should run command ```\python bin\storm blabla...```. --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
GitHub user HeartSaVioR opened a pull request: https://github.com/apache/storm/pull/280 STORM-487 Let bin/storm compatible with Windows I modified bin/storm to be compatible with Windows. bin/storm script didn't take care of OS specific chars, so I've modified it. ps. IntelliJ said many lines in storm script violates PEP8. I wish to fix it, but it could hide original diff, so I will fix it by other PR after merge this in. You can merge this pull request into a Git repository by running: $ git pull https://github.com/HeartSaVioR/storm STORM-487 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/280.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #280 commit ed4861d83357dcb13cd32621fa18d1d03497a8a0 Author: Jungtaek Lim Date: 2014-10-03T07:18:40Z STORM-487 Let bin/storm compatible with Windows --- 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 enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
