-
-
Notifications
You must be signed in to change notification settings - Fork 387
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
Remove uses of pkg_resources and distutils #1096
Remove uses of pkg_resources and distutils #1096
Conversation
00fe3d5
to
573c3e8
Compare
whoops, I missed that the CI jobs failed the first time. Now updated. |
39ac1f9
to
14a548e
Compare
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.
Thank you for working on this @diazona! All looks good just the one import fix.
pkg_resources is deprecated and, under some circumstances, causes import errors in Python 3.12. This commit replaces the deprecated pkg_resources functions that access distribution metadata with calls to corresponding functions from importlib.metadata (or its backport importlib_metadata) and packaging. This change will bring pyinfra more in line with modern recommendations and should resolve the errors. The change introduces a new dependency on importlib_metadata in Python versions prior to 3.10. Even though importlib.metadata was available in Python 3.8, its entry_points() function did not have the group keyword argument until 3.10, hence this commit prefers the backport until that Python version. The only remaining use of pkg_resources is the require() function, which will take more work to replace since there's no ready equivalent in any of the importlib* or packaging modules.
38d6d22
to
56e42be
Compare
pkg_resources is deprecated and, under some circumstances, causes import errors in Python 3.12. This commit replaces the one remaining function from pkg_resources, the require() function, with a new implementation that does the same thing. It was necessary to implement the function anew because there's no equivalent for require() in the packaging or importlib.* modules. There is an implementation in the hbutils package, but I didn't think it was worth bringing in a new dependency for just this one function. https://hansbug.github.io/hbutils/main/api_doc/system/python.html#check-reqs See also pypa/packaging-problems#664 This new implementation is based most closely on the original pkg_resources code.
This commit replaces distutils.spawn.find_executable() with shutil.which() from the standard library. distutils is deprecated and has been removed from Python 3.12+, causing import errors when pyinfra is run on that version.
56e42be
to
3985b7e
Compare
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.
Thank you once again @diazona, this is fantastic!
This PR replaces uses of deprecated APIs from
pkg_resources
anddistutils
with modern equivalents fromimportlib.*
,packaging
, and the standard library'sshutil
. The one exception is that I added a new custom reimplementation ofpkg_resources.require()
because there's no equivalent to that inimportlib.*
orpackaging
(or the standard library).I wasn't sure if there's any tests that would make sense to add here, since this PR isn't changing any functionality, and the only way I know of to reproduce the corresponding bug (#1088) seems to be installing pyinfra itself using pipx in Python 3.12 (and even then, it depends on some other environmental factors I haven't figured out yet), so it would be difficult to come up with a test that exercises that. Happy to take any suggestions if there's a test that would make sense to add here.
I note that the GitHub Actions workflow currently doesn't test pyinfra on Python 3.12, which would probably be good to add, but I'd consider that out of scope for this PR.
This should resolve #1088.