kewlfft / ansible-aur

Ansible module to manage packages from the AUR
GNU General Public License v3.0
299 stars 47 forks source link

Drop dependency on ansible (six.moves) #4

Closed panchoh closed 6 years ago

panchoh commented 6 years ago

As it is, aur.py depends on ansible being installed on the target host. So, you would need to get ansible installed in your target host just to be able to make use of aur.py and install whatever packages you need.

Let's review the dependency:

  from ansible.module_utils import six from six.moves import urllib

I reckon that the six¹ package is there to isolate an ansible module's code from the particular major version of python being used (2 or 3).

Fortunately, since last week², the ansible package in archlinux now depends directly on python3, so I reckon that in this particular instance, we can safely import urllib.request directly, which from now on will be spot on.

To recap, current arch ships python3, and the arch ansible package depends on it to run. aur.py's purpose is to run on archlinux hosts, thus, python3 can be safely assumed to be available. So we can depend directly on it and relax the dependency on ansible itself.

On a personal note, I am using this nice module to help me bootstrap my arch systems, so not having to install ansible just to be able to use it makes my day a little better :-)

Thanks a bunch!

[Feel free to edit the commit log]

kewlfft commented 6 years ago

Thanks for the PR. The import six was requested by some Python2 users. I have Ansible installed on the master only and it is running fine on the managed hosts. What are you getting because of this import? Is from ansible.module_utils.basic import * an issue as well?

Anyway because Ansible depends on python instead of python2 now I don't see any reason to keep this python2 support but I would like to understand your point first.

Thanks

panchoh commented 6 years ago

Hi there!

I just tried again, to make sure I hadn't dreamed it :-), and this is the error I got, when trying to use aur.py as it is now, when deploying a brand new host:

Traceback (most recent call last):\n  File \"/tmp/ansible_p8rvxrp4/ansible_module_aur.py\", line 6, in <module>\n    from six.moves import urllib\nModuleNotFoundError: No module named 'six'\ndebug3: mux_client_read_packet: read header failed: Broken pipe\r\ndebug2: Received exit status from master 1

Note the No module named 'six'.

Importing urllib.request instead, works like a charm.

Sorry for the delay. Long day.

Cheers!

kewlfft commented 6 years ago

What you seem to be missing is the python-six package on the managed node rather than to get Ansible installed. Can you confirm?

Some users use the module on hosts with Python2 only, even if they may have Python on the master (as it is now a dependency). By removing the compatibility tweak I am imposing a dependency to python, if I use the tweak I am imposing a dependency to python-six.

Can you therefore try another approach with

try:
    from urllib.request import urlopen
except ImportError:
    from urllib2 import urlopen
kewlfft commented 6 years ago

please try my new version

kewlfft commented 6 years ago

I think the new approach is clean and python version neutral let me know Thanks

panchoh commented 6 years ago

Testing it right now. Looks very nice to me! Thanks!

panchoh commented 6 years ago

Works like a charm. Thanks again! Closing...

kewlfft commented 6 years ago

Thanks for raising the issue