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
Allow manually defining user & hostname in callback plugin #496
Allow manually defining user & hostname in callback plugin #496
Conversation
In some environments (e.g. containers), the user and hostname detected from the OS might not be relevant or useful. This change allows the user to pass in customized values for these attributes which override the automatic detection.
Build succeeded. ✔️ ara-tox-py3 SUCCESS in 4m 30s |
Thanks for the PR!
Looks good to me, though I had a question in-line.
There's unit tests for playbook properties but it's more about the API than the callback plugin since a lot of it is mocked: https://github.com/ansible-community/ara/blob/master/ara/api/tests/tests_playbook.py For testing playbook properties as recorded by the callback we have integration tests written as Ansible playbooks like here: ara/tests/integration/lookups.yaml Lines 13 to 41 in 95176a9
That particular playbook is one of the test playbooks launched when running Adding coverage is always nice but not a hard requirement since we can take care of it in a follow up PR before release. |
Hi @phemmer and sorry it's taken so long. There is nothing wrong with the code and I've tested it locally successfully: I will merge this as-is and follow up with a patch for integration testing it. |
This will be released as part of ara 1.7.0 sometime in the near future. |
In some environments (e.g. containers), the user and hostname detected from the OS might not be relevant or useful. This change allows the user to pass in customized values for these attributes which override the automatic detection.
Note: I didn't see any tests for things like the the the controller name or user name, or any other playbook level attributes, so I didn't add any here.
Closes #493