[Freeipa-devel] Another batch of Python 3 patches

Thu May 5 14:45:17 UTC 2016

On 05.05.2016 15:53, Petr Viktorin wrote: 

On 05/04/2016 01:55 PM, Martin Basti wrote: 

<blockquote>

On 03.05.2016 18:14, Petr Viktorin wrote: 

<blockquote>

On 05/03/2016 04:31 PM, Martin Basti wrote: 

<blockquote>

On 03.05.2016 15:52, Petr Viktorin wrote: 

<blockquote>

On 05/03/2016 03:02 PM, Petr Spacek wrote: 

<blockquote>

On 2.5.2016 18:02, Martin Basti wrote: 

<blockquote>

On 29.04.2016 19:46, Petr Viktorin wrote: 

<blockquote>

Hello,
These patches concentrate on tests, and code that was added/changed
since I last looked at the FreeIPA project.

With these patches, I'm back to getting the same errors under py2
and
py3 when in test_xmlrpc. 

Patch 777:
Could you fix all relative imports and enable check in pylint for
that?
(Remove relative-import from pylintrc), IMO there is just one extra
relative
import in custodia module. 

</blockquote>

</blockquote>

Would it be OK if I do that in a separate patch, in the next batch?
This
one is fixing the tests.
(I have the change in my worktree, so I won't forget when I next sit
down to work on IPA.) 

</blockquote>

It is okay to send it in a next patch :)
ACK on this patch then 

<blockquote>

<blockquote>

<blockquote>

Do you plan to use in py2 ?
from__future__importabsolute_import 

</blockquote>

</blockquote>

I think that's unnecessary boilerplate; the errors this catches are
easily found by other means.
And it doesn't guard against someone forgetting the __future__ import
itself in a new file. The pylint check will be much better. 

</blockquote>

Ok, just FYI pylint has check that prevents forgetting this import
(disabled in IPA) 

<blockquote>

<blockquote>

<blockquote>

Patch 778:
LGTM

Patch 779
LGTM

Patch 780
LGTM

Patch 781
LGTM

Patch 782
Not sure, I will review it longer

Patch 783
LGTM

Patch 784
LGTM

Patch 785
LGTM

I will test it with both py2 and py3 to convert LGTM to ACK :) 

</blockquote>

Functional ACK, I did not find any breakage (when combined with other
Py3
patches). 

</blockquote>

</blockquote>

Hold your horses :D, I probably find something in tests

I run ipa-run-tests with xmlrpc tests under python2 and python3, please
note the different count of tests and errors in py3

platform linux2 -- Python 2.7.11, pytest-2.8.7, py-1.4.31, pluggy-0.3.1
-- /usr/bin/python
rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile: pytest.ini
collecting ... collected 1835 items

platform linux -- Python 3.5.1, pytest-2.9.1, py-1.4.31, pluggy-0.3.1 --
/bin/python3
rootdir: /usr/lib/python3.5/site-packages/ipatests, inifile: pytest.ini
collecting ... collected 1694 items / 7 errors

Collecting failed on following import errors:
    test_xmlrpc/test_add_remove_cert_cmd.py:13: in <module>
ipatests.test_xmlrpc.testcert import get_testcert
    xmlrpc/testcert.py:34: in <module>
ipaserver.plugins import rabase
    ImportError: No module named 'ipaserver'

And I found more errors, but they may be unrelated I have to investigate
more
Martin^2 

</blockquote>

Right, some of the xmlrpc tests use ipaserver, which isn't fully ported
yet, and the python3-ipaserver RPM isn't even built. The parts of
ipaserver needed for these tests will be my next goal. 

</blockquote>

OK

However I see errors under py3 in cert tests 

</blockquote>

[...]

For me, those tests don't pass under Python 2 either, but I'll keep looking.
Is this blocking the other patches? 

</blockquote>
Well I'm sure that under py2 those tests passed, and in jenkins they pass as well. 
I tested it without your patches and py3 tests failed with same errors as I reported, so ACK. (but would be nice to fix it :) ) 

Pushed to 

master: 
* f753ad322dfdd81907a309827bddfcb1e47917a7 test_xmlrpc: Use absolute imports 
* 6406c7a5935e9fb9cd41af49f67d6200021b3574 xmlrpc_test: Rename exception instance before working with it 
* 890f83b0bbd5ec03397e817ed1282fa66efab7da radiusproxy plugin: Use str(error) rather than error.message 
* 095d0cb7afc3d404829d87bc894d8691be2228ef xmlrpc_test: Expect bytes rather than strings for binary attributes 
* baaa041b8a272e43c99f00f69fc645a2e92216db ipalib.rpc: Send base64-encoded data as string under Python 3 
* 14aba1c7c16e3b4e6375305990fffbd86cefbfdd range plugin tests: Use bytes with MockLDAP under Python 3 
* bdee89001455825bfe2c7e820c6b2d651f1f45eb radiusproxy plugin tests: Expect bytes, not text, for ipatokenradiussecret 
* 6ddf0d657f70eb03d17af2e63eb52d6ef33305be certprofile plugin: Use binary mode for file with binary data 
* 20a6a42567f0fa21945f02aa58420c31bc0c9127 test_add_remove_cert_cmd: Use bytes for base64.b64encode() 

ipa-4-3: 
* 5750c3ece9359ca5a1c9ddbbf727eb63862709ce test_xmlrpc: Use absolute imports 
* ba3d77253a54e862249476e3d289ebf01f1a9f9a xmlrpc_test: Rename exception instance before working with it 
* a514ebdc81548f37e5fedb7bb43ca7ddab473af8 radiusproxy plugin: Use str(error) rather than error.message 
* 4c68bd671a89f3ddda0c09f9abffe0e1b87098cb xmlrpc_test: Expect bytes rather than strings for binary attributes 
* 72fb2674117ee8c3c1cafa17512e524ea437f966 ipalib.rpc: Send base64-encoded data as string under Python 3 
* 8f637bc9b1343979a0ead7346a5b0d771f133420 range plugin tests: Use bytes with MockLDAP under Python 3 
* aa052a0976ce3ab643fd54a10bc5428711dd9b61 radiusproxy plugin tests: Expect bytes, not text, for ipatokenradiussecret 
* ffb8fcc20848b570bff7a7776ad08d23ac882ff5 certprofile plugin: Use binary mode for file with binary data 
* c009ea8a7622a567de2101d8577955942e67a44d test_add_remove_cert_cmd: Use bytes for base64.b64encode() 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160505/7f9f99d0/attachment.htm>