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

update cfg mgr exit method #92

Merged
merged 1 commit into from
Apr 15, 2024
Merged

Conversation

rekup
Copy link
Contributor

@rekup rekup commented Apr 13, 2024

When we use module.fail_json in our modules, this will cause our config managers __exit__ function to raise an exception. This will cause Ansible to display a warning (Module invocation had junk after the JSON data):

TASK [Converge - Set preserve logs] **************************************************************************************************************************
[WARNING]: Module invocation had junk after the JSON data: Exception occurred: 1
fatal: [opnsense]: FAILED! => {"changed": false, "msg": "Parameter max_log_file_size_mb is not supported in OPNsense 23.1"}

This is probably ok if the error was caused by an unhandled exception, but it's not what we expect if we handle an error in our module and deliberately call module.fail_json. This PR fixes the by raising any SystemExit exceptions "as is" which enables Ansible to correctly recognize the call to module.fail_json without displaying the warning.

Copy link
Contributor

@KiLLuuuhh KiLLuuuhh left a comment

Choose a reason for hiding this comment

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

looks good to me 👍

@rekup rekup merged commit aaa372e into puzzle:main Apr 15, 2024
36 checks passed
KiLLuuuhh pushed a commit to KiLLuuuhh/puzzle.opnsense that referenced this pull request Apr 16, 2024
KiLLuuuhh pushed a commit to KiLLuuuhh/puzzle.opnsense that referenced this pull request Apr 16, 2024
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