You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by HeartSaVioR <gi...@git.apache.org> on 2014/10/03 09:26:49 UTC
[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 <ka...@gmail.com>
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by harshach <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by HeartSaVioR <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by ChitturiPadma <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by ChitturiPadma <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by harshach <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by HeartSaVioR <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by harshach <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by harshach <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by ChitturiPadma <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by harshach <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by HeartSaVioR <gi...@git.apache.org>.
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 #<RuntimeException java.lang.RuntimeException: Failed to delete storm-local\workers\816a0fd6-7cdf-4312-a5ac-758f7f55ce46>
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 #<RuntimeException java.lang.RuntimeException: Failed to delete storm-local\workers\816a0fd6-7cdf-4312-a5ac-758f7f55ce46>
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 #<RuntimeException java.lang.RuntimeException: Failed to delete storm-local\workers\c26b2e31-d006-4d4d-9e56-45ca9391018d>
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 #<RuntimeException java.lang.RuntimeException: Failed to delete storm-local\workers\c26b2e31-d006-4d4d-9e56-45ca9391018d>
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 clearing state for id c26b2e31-d006-4d4d-9e56-45ca9391018d. 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:c26b2e31-d006-4d4d-9e56-45ca9391018d
2014-10-05 17:49:06 b.s.d.supervisor [INFO] Shut down 95111631-8f7f-46ed-a93a-8db1d01ae4e0:c26b2e31-d006-4d4d-9e56-45ca9391018d
```
I use storm-starter example.
```
C:\apache-storm-0.9.2-incubating\examples\storm-starter>python ..\..\bin\storm j
ar storm-starter-topologies-0.9.2-incubating.jar storm.starter.WordCountTopology
production-topology remote
```
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by asfgit <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by HeartSaVioR <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by dashengju <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by revans2 <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by HeartSaVioR <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by HeartSaVioR <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by caofangkun <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/280#issuecomment-57764337
In Windows, users should run command ```<PYTHON_PATH>\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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by HeartSaVioR <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by HeartSaVioR <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by ptgoetz <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by ChitturiPadma <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by HeartSaVioR <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-487 Let bin/storm compatible with Window...
Posted by dashengju <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---