Worked on this during the holidays. This needs a test if we agree with the general approach

Must be changed to specific version of Python Interpreter. #!/usr/bin/python3

We will be asked to remove this line. Internal RH names are prohibited to go public. Just a warning. I was asked to remove internal links a few times before.

Not sure, why independent function makes assumption that parent directory was created for before its call.

Please, go to Python 3, and do not import individual functions. This harms code standards.

Make urlopen as part of corresponding module. urllib.request.urlopen() or request.urlopen()

I wonder, if it is more appropriate use logger. logger can output code to stderr too.

In upstream code no place for such commented lines.

Just, a quick question: does this script will correctly process sources file with multi-lines, such as: https://src.fedoraproject.org/rpms/kernel/blob/master/f/sources ?

Definitely I would move above Ansible plugin to pre-defined path for third-part plugins dir and make in available to all system.

Must be changed to specific version of Python Interpreter. #!/usr/bin/python3

Can I drop python2 support all together? If so, that'd be great!

Definitely I would move above Ansible plugin to pre-defined path for third-part plugins dir and make in available to all system.

I think this module should only be exposed as a role. Ansible discourages writing modules, and roles are the preferred form for interacting modules. Lets move this into the pre-defined path once there are more than one role that needs to call this module.

Just, a quick question: does this script will correctly process sources file with multi-lines, such as: https://src.fedoraproject.org/rpms/kernel/blob/master/f/sources ?

Yes, it's written like that, but we can make sure it works with a test.

In upstream code no place for such commented lines.

Will add a verbose option.

Not sure, why independent function makes assumption that parent directory was created for before its call.

This is pretty normal, but I'll change it if it helps get this merged ;)

python3 doesn't work for a ansible module, at least not on Fedora 29. The module appears to be loaded in python2 directly. It is not executed as a script. I have verified this, and marked the source-lookaside.py file as executable ... but it doesn't help.

Therefore I am retaining python2/python3 support in the source-lookaside.py file.

Any ideas how can I resolve this while remaining true to the Standard Test Interface, which perscribes invoking "ansible-playbook" as the entry point?

Make urlopen as part of corresponding module. urllib.request.urlopen() or request.urlopen()

Until we are able to use python3 modules via the standard 'ansible-playbook' command, I don't believe I can fix this.

Must be changed to specific version of Python Interpreter. #!/usr/bin/python3

Can I drop python2 support all together? If so, that'd be great!

Yes, we should pick up only Python3. Python2 is deprecated in Fedora now.

python3 doesn't work for a ansible module, at least not on Fedora 29. The module appears to be loaded in python2 directly. It is not executed as a script. I have verified this, and marked the source-lookaside.py file as executable ... but it doesn't help.
Therefore I am retaining python2/python3 support in the source-lookaside.py file.
Any ideas how can I resolve this while remaining true to the Standard Test Interface, which perscribes invoking "ansible-playbook" as the entry point?

Make urlopen as part of corresponding module. urllib.request.urlopen() or request.urlopen()

Until we are able to use python3 modules via the standard 'ansible-playbook' command, I don't believe I can fix this.

It should work.
Please make sure you call ansible-3 or ansible-playbook-3

It should work. Please make sure you call ansible-3 or ansible-playbook-3

Neither the standard test interface, nor developer documentation, describes calling "ansible-3" or "ansible-playbook-3". Until it does (and requires python3 and ansible3 everywhere), we should retain 2 and 3 compatibility.

All other review comments have been addressed.

It should work. Please make sure you call ansible-3 or ansible-playbook-3

The standard test interface does not describe calling "ansible-3" or "ansible-playbook-3". Until it does (and thus requires python3 and ansible3), we should retain 2 and 3 compatibility.
All other review comments have been addressed.

Personally I have opposite view.
Python2 is officially deprecated in Fedora.
All Fedora community moved toward Python3.
Python2 is NOT installed by default.
There is Ansible build that uses only Python3.

In my understanding STI says: run Ansible playbook.
STI doesn't mention about Python2.
Moreover, if Ansible command will always stay as ansible-3 and ansible-playbook-3 than you doom STI for using Python2 forever. Binding STI to specific Python version is bad.

Up to now, current master uses ONLY Python3.
There is released version 3.0 STR that uses only Python3.
With this approach you suggest to install in system two Pythons.
My point, let's be flexible, modern, and follow community tendencies.
I announced to mail list that STR was completely migrated to Python3.
All feedback was only positive.
My point: we should retain 2 and 3 compatibility. --- no, we should not.
In current Fedora/RHEL8 you MUST specify /usr/bin/python3
For me it is very strange to call deprecated Ansible for Python3 code.

Keeping this PR until receive some feedback from @dperpeet @bookwar @mvadkert @bgoncalv @plautrba @vdolezal or other people.

My point: we should retain 2 and 3 compatibility. --- no, we should not.
In current Fedora/RHEL8 you MUST specify /usr/bin/python3
For me it is very strange to call deprecated Ansible for Python3 code.

I'm with you: that we should move to python3. Happy you're planning ahead.

However, there is a lot of work necessary (to update documentation and reeducate maintainers) before the Standard Test Interface supports only Python3.

All of these document python 2 and ansible with python2:
* Landing page: https://fedoraproject.org/wiki/CI
* Our documented interface: https://fedoraproject.org/wiki/Changes/InvokingTests
* Our documented tutorials: https://fedoraproject.org/wiki/CI/Quick_Start_Guide
* Our test creation guide: https://fedoraproject.org/wiki/CI/Tests

Where is the work to migrate to python 3 being tracked?

As a real example, if I were to drop python 2 from this pull request, the standard-test-source role would just fail with a big exception for maintainers that are following our docs.

Again, I'm not against dropping python 2 support, and I'm really glad you're thinking ahead here. But that work must be done properly or it breaks the workflows of our users in an unacceptable way.

Checking which docs need to be updated. But to make it clear: Are we now supposed to use ansible-playbook-3 to run tests? There does not seem to be such binary in ansible-2.7.5-1.fc29.noarch. I've tested with ansible-playbook and it works fine.

@psss thank you, right

[root@host-8-254-147 standard-test-roles]# which ansible-playbook 
/usr/bin/ansible-playbook
[root@host-8-254-147 standard-test-roles]# rpm -qf /usr/bin/ansible-playbook
ansible-2.7.5-1.fc29.noarch
[root@host-8-254-147 standard-test-roles]# rpm -qR ansible
/usr/bin/python3
config(ansible) = 2.7.5-1.fc29
python(abi) = 3.7
python3-PyYAML
python3-crypto
python3-jinja2
python3-jmespath
python3-paramiko
python3-setuptools
python3-six
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PartialHardlinkSets) <= 4.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(PayloadIsXz) <= 5.2-1
sshpass

All are Python3

Checking which docs need to be updated. But to make it clear: Are we now supposed to use ansible-playbook-3 to run tests? There does not seem to be such binary in ansible-2.7.5-1.fc29.noarch. I've tested with ansible-playbook and it works fine.

It works fine with the pull request as posted. But when I make the following changes (to drop python2 support) it fails with this exception:

--- a/roles/standard-test-source/library/source-lookaside.py
+++ b/roles/standard-test-source/library/source-lookaside.py
@@ -33,13 +33,9 @@ import re
 import os
 import shutil
-try:
-    from urllib.request import urlopen
-    from urllib.error import URLError
-    from configparser import ConfigParser
-except ImportError:
-    from urllib2 import urlopen, URLError
-    from ConfigParser import ConfigParser
+import urllib.request
+import urllib.error
+import configparser
 ANSIBLE_METADATA = {
     'metadata_version': '1.1',
@@ -125,7 +121,7 @@ logger.setLevel(logging.INFO)
 def lookasides():
     for url in LOOKASIDES:
         yield url + LOOKASIDE_URI
-    config = ConfigParser()
+    config = configparser.ConfigParser()
     config.read(glob.glob(LOOKASIDE_CONFIG))
     for section in config.sections():
         if config.has_option(section, "lookaside"):
@@ -164,8 +160,8 @@ def retrieve(url, target):
     try:
         with open(dest, 'wb') as fp:
-            shutil.copyfileobj(urlopen(url), fp)
-    except URLError as ex:
+            shutil.copyfileobj(urllib.request.urlopen(url), fp)
+    except urllib.error.URLError as ex:
         if not hasattr(ex, "code") or ex.code != 404:
             logger.error("{0}: {1} {2}".format(name, url, str(ex)))
         return None

With this exception:

MODULE FAILURE
See stdout/stderr for the exact error
MODULE_STDERR:
Traceback (most recent call last):
  File "/root/.ansible/tmp/ansible-tmp-1547728140.5455825-143940266836305/AnsiballZ_source-lookaside.py", line 113, in <module>
    _ansiballz_main()
  File "/root/.ansible/tmp/ansible-tmp-1547728140.5455825-143940266836305/AnsiballZ_source-lookaside.py", line 105, in _ansiballz_main
    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)
  File "/root/.ansible/tmp/ansible-tmp-1547728140.5455825-143940266836305/AnsiballZ_source-lookaside.py", line 48, in invoke_module
    imp.load_module('__main__', mod, module, MOD_DESC)
  File "/tmp/ansible_source-lookaside_payload_s2CL_w/__main__.py", line 36, in <module>
ImportError: No module named request

This is because:

$ python2
Python 2.7.15 (default, Oct 15 2018, 15:26:09) 
[GCC 8.2.1 20180801 (Red Hat 8.2.1-2)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import urllib.request
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: No module named request

Whereas python3 works fine. Hence I added the python 2/3 try/import stuff in the original pull request.

I've pushed the above patch as a FIXUP in this pull request. So you can see if you can reproduce the issue. I'm running on:

• Fedora 29
• ansible-2.7.5-1.fc29.noarch

And I run this command to reproduce the problem:

sudo ansible-playbook tests/source.yml

Or run tests.yml in any dist-git repo that uses standard-roles-source ... to also reproduce.

@stefw you are right.

Ansible plugins must be both Python2/Python3 compatible.

# ansible all -i localhost, -m setup --extra-vars "ansible_connection=local" | grep python
            "_": "/usr/bin/python"
        "ansible_python": {
            "executable": "/usr/bin/python",
        "ansible_python_version": "2.7.15",
        "ansible_selinux_python_present": false,

And, even Anisible-engine runs with help of Python3 all actions are done with help of Python2.

Could you please bring back compatibility with Python2 only plugin code? Thank you for your time.

Thank you. Force pushed. Hopefully this is ready to go in now.

Commit 806aa1a0 fixes this pull-request

pretty please pagure-ci rebuild