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

use os._exit instead of sys.exit when daemonizing #49303

Merged
merged 1 commit into from Aug 24, 2018

Conversation

Projects
None yet
4 participants
@gtmanfred
Contributor

gtmanfred commented Aug 24, 2018

What does this PR do?

According to the docs here

os._exit should should normally only be used in the child process after a fork().

This will exit the process with status n, without calling cleanup
handlers, flushing stdio buffers, etc.

do not merge, this is just testing to see if it breaks any other tests for python<3.7 at least.

What issues does this PR fix or reference?

Fixes saltstack/salt-jenkins#1075

Tests written?

Yes

Commits signed with GPG?

Yes

use os._exit instead of sys.exit when daemonizing
According to the docs [here](https://docs.python.org/3/library/os.html#os._exit)

> `os._exit` should should normally only be used in the child process after a fork().

This will exit the process with status n, without calling cleanup
handlers, flushing stdio buffers, etc.

Fixes saltstack/salt-jenkins#1075

@gtmanfred gtmanfred requested a review from saltstack/team-core Aug 24, 2018

@gtmanfred

This comment has been minimized.

Contributor

gtmanfred commented Aug 24, 2018

requesting reviews just in case someone knows why we might not want to use this function

@gtmanfred gtmanfred changed the title from [DO NOT MERGE] use os._exit instead of sys.exit when daemonizing to use os._exit instead of sys.exit when daemonizing Aug 24, 2018

@dwoz

dwoz approved these changes Aug 24, 2018

Looks good to me; the python-daemon package also does this:

https://github.com/arnaudsj/python-daemon/blob/master/daemon/daemon.py#L573

@rallytime rallytime requested review from terminalmage and cachedout Aug 24, 2018

@gtmanfred

This comment has been minimized.

Contributor

gtmanfred commented Aug 24, 2018

@gtmanfred gtmanfred merged commit 4c81c2e into saltstack:2018.3 Aug 24, 2018

8 checks passed

WIP ready for review
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
jenkins/pr/docs The docs job has passed
Details
jenkins/pr/lint The lint job has passed
Details
jenkins/pr/py2-centos-7 The py2-centos-7 job has passed
Details
jenkins/pr/py2-ubuntu-1604 The py2-ubuntu-1604 job has passed
Details
jenkins/pr/py3-centos-7 The py3-centos-7 job has passed
Details
jenkins/pr/py3-ubuntu-1604 The py3-ubuntu-1604 job has passed
Details

@gtmanfred gtmanfred deleted the gtmanfred:forking branch Aug 24, 2018

rallytime added a commit that referenced this pull request Aug 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment