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

Fixes #5159 file module: Don't catch SystemExit #5160

Merged
merged 3 commits into from
Dec 16, 2013

Conversation

JensRantil
Copy link
Contributor

This fixes issue #5159.

@jctanner
Copy link
Contributor

@JensRantil it would be nice to include the content of "e" in the exit message.

@JensRantil
Copy link
Contributor Author

@JensRantil it would be nice to include the content of "e" in the exit message.

@jctanner Good point. Absolutely. Is there a standard keyword for that? Will I need to translate e to a string before setting it as a parameter to module.exit_json(...)?

Also, maybe module.fail_json(...) is a better function to call?

@jctanner
Copy link
Contributor

You can cast e to a string:

module.exit_json(msg="rmtree failed: %s" % str(e))

Yes, fail_json would be a better way to exit if there is a true exception.

@JensRantil
Copy link
Contributor Author

@jctanner Fixed. Holler if you have any other corrections.

jctanner added a commit that referenced this pull request Dec 16, 2013
Fixes #5159 `file` module: Don't catch `SystemExit`
@jctanner jctanner merged commit 53a3671 into ansible:devel Dec 16, 2013
@jctanner
Copy link
Contributor

@JensRantil thank you!

Tested with:

# prepatch, delete dir check
jtanner@u1304:~/issues/5159-file-directory-exception$ ansible -i inventory localhost --check -m file -a 'path=/tmp/foo state=absent'
localhost | FAILED => failed to parse: {"changed": true}
{"msg": "rmtree failed", "changed": false}

# directory owned by jtanner, remove as root
jtanner@u1304:~/issues/5159-file-directory-exception$ ansible -i inventory localhost --check -m file -a 'path=/tmp/foo state=absent'

localhost | success >> {
    "changed": true
}

# directory owned by root, delete as jtanner
jtanner@u1304:~/issues/5159-file-directory-exception$ ansible -i inventory localhost  -c local -m file -a 'path=/tmp/foo state=absent'
localhost | FAILED >> {
    "failed": true,
    "msg": "rmtree failed: [Errno 1] Operation not permitted: '/tmp/foo'"
}

jctanner added a commit that referenced this pull request Dec 19, 2013
Fixes #5159 `file` module: Don't catch `SystemExit`
@ansibot ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 5, 2018
@ansible ansible locked and limited conversation to collaborators Apr 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue/PR relates to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants