-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Replace ansible.module_utils._text by ansible.module_utils.common.text.converters #2877
Replace ansible.module_utils._text by ansible.module_utils.common.text.converters #2877
Conversation
@felixfontein This PR was evaluated as a potentially problematic PR for the following reasons:
Such PR can only be merged by human. Contact a Core team member to review this PR on IRC: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look OK, but a quick grep through source code revealed some potential misses:
$ git grep "module_utils._text"
tests/unit/mock/loader.py:12:from ansible.module_utils._text import to_bytes, to_text
tests/unit/mock/procenv.py:16:from ansible.module_utils._text import to_bytes
tests/unit/mock/vault_helper.py:6:from ansible.module_utils._text import to_bytes
tests/unit/plugins/module_utils/conftest.py:15:from ansible.module_utils._text import to_bytes
tests/unit/plugins/modules/conftest.py:12:from ansible.module_utils._text import to_bytes
tests/unit/plugins/modules/monitoring/test_circonus_annotation.py:14:from ansible.module_utils._text import to_bytes
tests/unit/plugins/modules/net_tools/test_nmcli.py:11:from ansible.module_utils._text import to_text
tests/unit/plugins/modules/packaging/os/test_rhn_register.py:11:from ansible.module_utils._text import to_native
tests/unit/plugins/modules/remote_management/lenovoxcc/test_xcc_redfish_command.py:11:from ansible.module_utils._text import to_bytes
tests/unit/plugins/modules/system/test_ufw.py:9:from ansible.module_utils._text import to_bytes
tests/unit/plugins/modules/utils.py:11:from ansible.module_utils._text import to_bytes
tests/unit/plugins/modules/web_infrastructure/test_jenkins_build.py:9:from ansible.module_utils._text import to_bytes
Were those left out of this changeset on purpose?
@tadeboro no they weren't... I only grepped in |
Thank you for this. Makes me wonder if we should have CI detect is people are using private methods |
Not all private methods are private in ansible/ansible, so I think this is not possible yet. |
Backport to stable-3: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply fafabed on top of patchback/backports/stable-3/fafabed9e6acc6bd49ce6e9bf266ee27f686aebe/pr-2877 Backporting merged PR #2877 into main
🤖 @patchback |
…t.converters (ansible-collections#2877) * Replace ansible.module_utils._text by ansible.module_utils.common.text.converters. * Also adjust tests. (cherry picked from commit fafabed)
Manual backport to stable-3 in #2882. There was a conflict in plugins/modules/cloud/scaleway/scaleway_security_group_rule.py due to removal of the vendored |
SUMMARY
While the private API
ansible.module_utils._text
was the default place to getto_text
,to_bytes
andto_native
for a long time, at least since Ansible 2.9 there's a non-private alternative:ansible.module_utils.common.text.converters
. So let's use it :)ISSUE TYPE
COMPONENT NAME
various plugins