Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

exec version #2

Closed
wants to merge 7 commits into from
Closed

exec version #2

wants to merge 7 commits into from

Conversation

czervenka
Copy link
Contributor

I have replaced system() call by python's exec() in templates/bin/python.py. This version of script si simpler, nicer and does work with programs like daemon tools, eclipse debugger, etc. There is no need for parsing arguments and also no extra useless process hanging in memory.

Tests were ok.


def inject_pythonpath():
'''
insert virtualevn path into pythonpath
'''
pathdelim = sys.platform == 'win32' and ';' or ':'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is alreadyos.path.pathsep for this purpose (tested on Linux yet, but should work on Win and Mac too).

@kvbik
Copy link
Owner

kvbik commented Sep 26, 2011

wonderful! thank you @czervenka

i will run the test suite on windows and some older pythons and let you know. i'd really like to merge this stuff!

didn't know about os.execvp. really like it ;)

@czervenka
Copy link
Contributor Author

I realized that Win needs special quoting for arguments with spaces. All tests passed on WindowsXP/python2.7 now.

kvbik pushed a commit that referenced this pull request Feb 6, 2012
kvbik pushed a commit that referenced this pull request Feb 6, 2012
@kvbik
Copy link
Owner

kvbik commented Feb 9, 2012

hey, so os.execvp stuff is in master branch. any volunteers for testing?:)

@czervenka
Copy link
Contributor Author

We have tried this version in our preproduction environment. I can confirm that there are no problems running daemontools rvirtualenv gunicorn with this "exec" version.

@kvbik
Copy link
Owner

kvbik commented Feb 15, 2012

@czervenka that's wonderfull news. thank you. i will have a quick look at #5 and do the release as soon as possible.

@kvbik
Copy link
Owner

kvbik commented Feb 15, 2012

but i believe this is ok.

@kvbik kvbik closed this Feb 15, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants