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

Stop using distutils #969

Closed
webknjaz opened this issue Jan 20, 2022 · 6 comments · Fixed by #976
Closed

Stop using distutils #969

webknjaz opened this issue Jan 20, 2022 · 6 comments · Fixed by #976
Assignees

Comments

@webknjaz
Copy link
Member

It's deprecated. Python 3.10 raises a DeprecationWarning and Python 3.12 won't have it in the stdlib anymore.
Why fix it now and not wait for another 2 years when this will produce an ImportError? Because it's nice to solve such problems ahead of time and also since ansible-runner is a library, it also causes these warnings (or even errors with pytest's filterwarning = error or python -Werror 1) for the projects that have Python 3.10 in their test matrixes. In particular, it's been observed in ansible-navigator.

Footnotes

  1. Learn more at https://docs.python.org/dev/using/cmdline.html#cmdoption-W

@github-actions github-actions bot added the needs_triage New item that needs to be triaged label Jan 20, 2022
@cidrblock
Copy link
Contributor

In case this might help

  File "<string>", line 1, in <module>
  File "/home/bthornto/github/ansible-navigator/src/ansible_navigator/runner/__init__.py", line 4, in <module>
    from .ansible_config import AnsibleConfig
  File "/home/bthornto/github/ansible-navigator/src/ansible_navigator/runner/ansible_config.py", line 8, in <module>
    from ansible_runner import get_ansible_config  # type: ignore[import]
  File "/home/bthornto/github/ansible-navigator/venv/lib64/python3.10/site-packages/ansible_runner/__init__.py", line 3, in <module>
    from .interface import run, run_async, \
  File "/home/bthornto/github/ansible-navigator/venv/lib64/python3.10/site-packages/ansible_runner/interface.py", line 27, in <module>
    from ansible_runner.config.runner import RunnerConfig
  File "/home/bthornto/github/ansible-navigator/venv/lib64/python3.10/site-packages/ansible_runner/config/runner.py", line 27, in <module>
    from distutils.dir_util import copy_tree
  File "/usr/lib64/python3.10/distutils/__init__.py", line 19, in <module>
    warnings.warn(_DEPRECATION_MESSAGE,
DeprecationWarning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives

@cidrblock
Copy link
Contributor

We were adding some code to navigator to check for ciruclar import before enabling isort and came across this. The distutils warning was suppressed, so it's not blocking anything, but since we came across it, thought it would be good to let you all now.

@AlanCoding
Copy link
Member

Do you have any good suggestion for an alternative to LooseVersion? I was not very happy with any that I saw.

@sivel
Copy link
Member

sivel commented Jan 21, 2022

Do you have any good suggestion for an alternative to LooseVersion? I was not very happy with any that I saw.

packaging.version is the recommended alternative. However, in core, we actually vendored distutils.version for now.

@AlanCoding
Copy link
Member

License-wise, can we add that module to runner (and then reuse in AWX)? We have this problem in a couple of projects.

https://github.com/ansible/ansible/blob/8d1cf7f266d7a81c248838917f8df5475e4439b9/lib/ansible/module_utils/compat/version.py

@Shrews
Copy link
Contributor

Shrews commented Jan 24, 2022

This should be somewhat simple to do. I see runner using distutils for the following cases:

  • distutils.spawn.find_executable() : We can replace this with shutil.which()
  • distutils.dir_util.copy_tree() : We can replace this with shutil.copytree(). Why the distutils version was even used is perplexing.
  • disutils.version.LooseVersion() : This is only used in our test suite to skip certain tests when run using a version of Ansible that is less than 2.8. Our tests use only 2.9 or higher, so we could just rip this out.

@Shrews Shrews self-assigned this Jan 24, 2022
@Shrews Shrews removed the needs_triage New item that needs to be triaged label Jan 25, 2022
ansible-zuul bot pushed a commit that referenced this issue Jan 31, 2022
Remove use of distutils

Fixes #969

Reviewed-by: Alexander Sowitzki <dev@eqrx.net>
Reviewed-by: None <None>
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 a pull request may close this issue.

5 participants