#152 python3: improve Python 3.x compatibility
Closed by tkopecek. Opened by pavlix.
pavlix/koji python3  into  master

Download 152.patch

Fix a number of Python 3.x issues. With this patch and the respective
patches in other packages I can successfully run at least some koji
commands using both Python 2.x and Python 3.x.

$ python -V
Python 3.4.3
$ koji moshimoshi
Warning: Could not install krbV module. Kerberos support will be disabled.
hej, pavlix!
You are using the hub at http://koji.fedoraproject.org/kojihub
Authenticated via client certificate /home/pavlix/.fedora.cert

Related: https://pagure.io/koji/issue/151

Signed-off-by: Pavel Šimerda pavlix@pavlix.net

rebased

rebased

rebased

Just updated the patchset using the six module. Is there anything else I can do for you? Part of the work can probably by also done using modernize so the rest would become a smaller patch then. Or the whole changeset can be applied at once.

What we need to check is whether I managed to fix all occurences of the renamed modules but otherwise the change should be good.

I took a quick look and the changes look good to me. I'd recommend eventually updating the %s formatting to use {}'s as well, but that can always be done later.

As mentioned in the issue you filed (#151), the p3 port is on our radar. There is some partial work here:

https://github.com/mikem23/koji-playground/tree/py3

I feel like using python-six and python-modernize is the way to go. However, there are a number of snags:

  1. we can't yet drop rhel5 support (2.4.3 means no python-six)
  2. ssl auth
  3. krb auth
  4. some other assorted libs we use

All together this means there is very little I can accept right this moment. Odds are we'll have to do something about snag no. 1 first before we can even consider taking changes along this line.

Also, currently this patch breaks a few unit tests

@mikem I know it was on your radar but I decided to make it happen so I can already use a patched version myself. As far as I see the rhel5/python-2.4.3 support is pretty much limiting as it doesn't even support the syntax used by 3.x and latest 2.x versions.

  • I was going to use python-modernize and rebase the patch on top of modernized code to split out changes that need to be done manually. But I'm not sure it's worth doing if you're not merging the changes anyway due to Python 2.4.3 compatibility.
  • Let's see what issues we get when using the modified code. So far I have checked basic koji and fedpkg operation. Libraries or unit tests won't be IMO so bad either.

I will be happy for anything that will make it easier for me to keep the patch working. I was already forced to modify it due to new changes in upstream code. But that's really hard as long as you need to keep 2.4.3 compatibility. What is the rationale by the way? Couldn't rhel5 just stick with whatever verision it's using? Tools like fedpkg/rpkg/rhpkg upstreams don't seem to keep 2.4.3 compatibility either.

@mikem Don't you have the ability to use python26 from EPEL for EL5? That should enable usage of python-six.

@mikem Don't you have the ability to use python26 from EPEL for EL5? That should enable usage of python-six.

Not only python-six. Python 2.4.3 cannot even parse the syntax that is required by Python 3.x, so you do need Python 2.6 or 2.7 to actually use Python 3.x friendly code.

rebased

9 new commits added

  • python3: fix up binary/text string check
  • python3: extend SSL code to support Python 3.x
  • python3: pass an empty bytestring to adler32
  • python3: fix up composite type usage after modernize
  • python3: fix up umask syntax after modernize
  • python3: fix up imports after modernize
  • python3: fix Makefile python checks
  • trivial: use ImportError directly
  • python3: modernize all client code

9 new commits added

  • python3: extend SSL code to support Python 3.x
  • python3: fix up binary/text string check
  • python3: pass an empty bytestring to adler32
  • python3: fix up composite type usage after modernize
  • python3: fix up umask syntax after modernize
  • python3: fix up imports after modernize
  • python3: fix Makefile python checks
  • trivial: use ImportError directly
  • python3: modernize all client code

I started using modernize for the client code to simplify the manually created patchset. As modernize is not perfect, I had to manually fix up the results and then make other manual changes. I carefully structured the changes into multiple commits.

Some of the commits could already be adapted to the current code base but most of them are based on the results of modernize.

rebased

rebased

rebased

@pingou I'm surprised that the comments tab no longer shows the current list of commits. The last one is 19 days ago and doesn't reflext the current patchset.

@pavlix, I'm seeing 11 commits from 2 days ago. Do you expect something else?

Sometimes it can be only dict, not module. It needs to be handled also as it gives me following error:

File "***/koji/koji/init.py", line 1927, in krb_login
raise builtins.ImportError("Please install python-krbV to use kerberos.")
AttributeError: 'dict' object has no attribute 'ImportError'

importlib.find_module is not available in python 2

I'm not able to run tests correctly on python 2.7 now. I've made inline comments to koji/__init__.py

@pavlix, I'm seeing 11 commits from 2 days ago. Do you expect something else?

@pingou I'm talking about that “9 new commits added” message.

11 new commits added

  • python3: fix composite type usage after modernize
  • python3: avoid deprecated imp module warning
  • python3: fix imports after modernize
  • python3: modernize all client code
  • python3: port umask octal numerals
  • python3: port binary/text string checks
  • python3: port adler32 numeric literals
  • python3: use binary string for adler32 input
  • python3: port SSLConnection code
  • python3: port Makefile python checks
  • python3: avoid using exceptions module

I'm not able to run tests correctly on python 2.7 now. I've made inline comments to koji/init.py

I don't have the chance to run the tests right now. I updated the patch to use six.moves for find_module() and load_module().

@pavlix, I'm seeing 11 commits from 2 days ago. Do you expect something else?

@pingou I'm talking about that “9 new commits added” message.

Did you push 9 commits in addition to 2 commits that were there before?

It looks like the notification picked up all 11 commits fine this time.

@pingou I'm talking about that “9 new commits added” message.

Did you push 9 commits in addition to 2 commits that were there before?

The commit messages differed as well. All messages had python3: prefix at the time.

It looks like the notification picked up all 11 commits fine this time.

Yep. Looks like all has been settled.

@pavlix Can you rebase against the current master branch?

@pavlix Can you rebase against the current master branch?

@ngompa Done. I previously somehow relied on git fetch but git fetch origin was needed.

rebased

@pavlix What's still left in terms of making Koji Python 3 compatible?

@pavlix What's still left in terms of making Koji Python 3 compatible?

1) My changes only affect the client and the library.

2) Apart from modernize I only made changes needed for the commands I actually run like koji moshimoshi or fedpkg build --scratch --srpm. I cannot currently run tests on my system due to some missing dependencies.

3) I'm still getting the following warning:

/usr/lib64/python3.4/site-packages/koji/ssl/SSLConnection.py:112: DeprecationWarning: str for buf is no longer accepted, use bytes
  sent = con.send(data, flags)

@mikem, @mikeb, @ralph: Can any of you take a look at this to see where it stands for merging?

@mikem, @mikeb, @ralph: Can any of you take a look at this to see where it stands for merging?

Any attempt to write code for Python 2.x and Python 3.x basically ends up requiring at least Python 2.6. So the merger is effectively blocked until the requirement to support Python 2.4.3 is lifted.

rebased

12 new commits added

  • use six.moves.builtins instead of builtins
  • avoid specfile warnings
  • python3: fix composite type usage after modernize
  • python3: fix imports after modernize
  • python3: modernize all client code
  • python3: port umask octal numerals
  • python3: port binary/text string checks
  • python3: port adler32 numeric literals
  • python3: use binary string for adler32 input
  • python3: port SSLConnection code
  • python3: port Makefile python checks
  • python3: avoid using exceptions module

Latest update works correctly with fedora infrastructure on Kerberos with Python 3.x and requests-kerberos, as long as you use one of the workarounds mentioned in #288.

rebased

rebased

We've merged PR #417 which obsoletes this one.

Pull-Request has been closed by tkopecek

Metadata