#313 Make the test suite pass on Python3
Merged by simo. Opened by puiterwijk.
puiterwijk/ipsilon py3_everything  into  master

Download 313.patch

With this patch set, the code will all be python3 compatible, and the container tests for Fedora 29 will run on an a system with python2 removed entirely.
Fedora 28 and CentOS will still be using Python2.

Note that this is based on top of #312.

rebased onto 866f7b6dc12379fdb9d8794b98f8a5af9df87221

Can't we also revert a4abfb230e9fd2a90959486461d63dcae81c7d3c now, since it'll be able to use Python 3 where FreeIPA is only offers Python 3?

Technically, sure.
But if the CLI gives us what we need, I don't think it gains us anything useful.

pycodestyle-3 (from python3-pycodestyle) is the actively maintained fork of pep8.

Aside from the pep8/pycodestyle thing, this looks good to me, though I'm concerned about the lack of test results returned from Jenkins. Doesn't this have CI?

pretty please pagure-ci rebuild

pretty please pagure-ci rebuild

@abbra unfortunately, it looks like there's no ci.centos.org pipeline wired up anymore, so it needs to be reviewed and tested locally.

Sorted returns a list, so the iterkeys here sounds redundant, plain sorted(opts) would work the same (faster maybe).

This will probably break in Python 4. Why not just else?

I generally see a lot of six.iteritems() and friends for small dicts (such as kwargs). Maybe this is overkill? If you just use .items() you get the proper behavior on Python 3 and the slightly worse behavior on Python 2 without noticing any difference. But the code will be more readable.

The omnipresent conditional decoding/encoding also feels like the str/bytes distinction wasn't given much thoughts. I'd consider imagining the data flow and agree whether you want to work with sequence of bytes or text and only do encoding/decoding at boundary level. However if you are confident in your tests, trying to pretend they are interchangeable (sticking with the wrong Python 2 paradigm) will work as well. Sorry if this sounds pedantic, I was bitten by this before, I merely try to share the experience.

Excellent read: https://portingguide.readthedocs.io/en/latest/strings.html

How can we move this forward?

Is anybody out there?

Hi Miro, I will try to take a look soon, the patchset is huge ...

@simo Have you had a chance to look over this? I'm starting to work on shipping Ipsilon with this patchset integrated for openSUSE because we're in Python 3 only land there...

Sorry, not yet, working vacation happened in the middle.

what's the point of this raise ?

Except for the extraneous raise all look good to me.
@puiterwijk if you can fix it I think we can merge

@puiterwijk do you want me to take over and fixe the raise as a follow commit ?

@simo Can you please just do it? I spoke to @puiterwijk at Flock about it and he wants to get this merged and a new release of Ipsilon with Python 3 support out the door.

I also will be contributing a spec for building it for SUSE distributions...

Ok, I am going to merge and then follow up with any changes as needed

Pull-Request has been merged by simo

Metadata