Re: [Freeipa-devel] Another batch of Python 3 patches
On Wed, 15 Jun 2016, Petr Spacek wrote: 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() I just found out one other nit: $ ipa help topics prints some commands as b'command' instead of plain command: $ ipa help topics automount b'automember' Auto Membership Rule. b'automount' Automount These were the ones (and more) I reported in https://fedorahosted.org/freeipa/ticket/5940 -- / Alexander Bokovoy -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Another batch of Python 3 patches
> 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() I just found out one other nit: $ ipa help topics prints some commands as b'command' instead of plain command: $ ipa help topics automount b'automember' Auto Membership Rule. b'automount' Automount -- Petr^2 Spacek -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Another batch of Python 3 patches
On 05.05.2016 15:53, Petr Viktorin wrote: On 05/04/2016 01:55 PM, Martin Basti wrote: On 03.05.2016 18:14, Petr Viktorin wrote: On 05/03/2016 04:31 PM, Martin Basti wrote: On 03.05.2016 15:52, Petr Viktorin wrote: On 05/03/2016 03:02 PM, Petr Spacek wrote: On 2.5.2016 18:02, Martin Basti wrote: On 29.04.2016 19:46, Petr Viktorin wrote: 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. 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.) It is okay to send it in a next patch :) ACK on this patch then Do you plan to use in py2 ? from__future__importabsolute_import 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. Ok, just FYI pylint has check that prevents forgetting this import (disabled in IPA) 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 :) Functional ACK, I did not find any breakage (when combined with other Py3 patches). 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 ipatests.test_xmlrpc.testcert import get_testcert xmlrpc/testcert.py:34: in 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 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. OK However I see errors under py3 in cert tests [...] For me, those tests don't pass under Python 2 either, but I'll keep looking. Is this blocking the other patches? 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
Re: [Freeipa-devel] Another batch of Python 3 patches
On 05/04/2016 01:55 PM, Martin Basti wrote: > > > On 03.05.2016 18:14, Petr Viktorin wrote: >> On 05/03/2016 04:31 PM, Martin Basti wrote: >>> >>> On 03.05.2016 15:52, Petr Viktorin wrote: On 05/03/2016 03:02 PM, Petr Spacek wrote: > On 2.5.2016 18:02, Martin Basti wrote: >> On 29.04.2016 19:46, Petr Viktorin wrote: >>> 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. 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.) >>> It is okay to send it in a next patch :) >>> ACK on this patch then >> Do you plan to use in py2 ? >> from__future__importabsolute_import 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. >>> Ok, just FYI pylint has check that prevents forgetting this import >>> (disabled in IPA) >>> >> 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 :) > Functional ACK, I did not find any breakage (when combined with other > Py3 > patches). > >>> 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 >>> ipatests.test_xmlrpc.testcert import get_testcert >>> xmlrpc/testcert.py:34: in >>> 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 >> 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. >> > OK > > However I see errors under py3 in cert tests > > [...] For me, those tests don't pass under Python 2 either, but I'll keep looking. Is this blocking the other patches? -- Petr Viktorin -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Another batch of Python 3 patches
On 03.05.2016 18:14, Petr Viktorin wrote:
On 05/03/2016 04:31 PM, Martin Basti wrote:
On 03.05.2016 15:52, Petr Viktorin wrote:
On 05/03/2016 03:02 PM, Petr Spacek wrote:
On 2.5.2016 18:02, Martin Basti wrote:
On 29.04.2016 19:46, Petr Viktorin wrote:
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.
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.)
It is okay to send it in a next patch :)
ACK on this patch then
Do you plan to use in py2 ?
from__future__importabsolute_import
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.
Ok, just FYI pylint has check that prevents forgetting this import
(disabled in IPA)
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 :)
Functional ACK, I did not find any breakage (when combined with other
Py3
patches).
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
ipatests.test_xmlrpc.testcert import get_testcert
xmlrpc/testcert.py:34: in
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
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.
OK
However I see errors under py3 in cert tests
=== FAILURES
===
___ test_cert.test_0003_service_show
___
self = 0x7f330af515c0>
def test_0003_service_show(self):
"""
Verify that service-show has the right certificate using
service-show.
"""
global cert
res = api.Command['service_show'](self.service_princ)['result']
> assert base64.b64encode(res['usercertificate'][0]) == cert
E assert
b'MIIEoDCCA4igAwIBAgIBEjANBgkqhkiG9w0BAQsFADBRMS8wLQYDVQQKDCZET00tMDEyLkFCQy5JRE0uTEFCLkVORy5CUlEuUkVESEFULkNPTTEeMBwG...upSCr/8sWoLTmDxcmiE0DhXfCqatdLBBN2AoZi0dx3Md07Pn+8wrmYW+ZXfheMZlrVaTg+bhfBq
x4kuL22dynWP46b1gCKbBu4a7vYZ3tsi3ZMJW9QKauk' ==
'MIIEoDCCA4igAwIBAgIBEjANBgkqhkiG9w0BAQsFADBRMS8wLQYDVQQKDCZET00tMDEyLkFCQy5JRE0uTEFCLkVORy5CUlEuUkVESEFULkNPTTEeMBwGA...upSCr/8sWoLTmDxcmiE0DhXfCqatdLBBN2AoZi0dx3
Md07Pn+8wrmYW+ZXfheMZlrVaTg+bhfBqx4kuL22dynWP46b1gCKbBu4a7vYZ3tsi3ZMJW9QKauk'
E+ where
b'MIIEoDCCA4igAwIBAgIBEjANBgkqhkiG9w0BAQsFADBRMS8wLQYDVQQKDCZET00tMDEyLkFCQy5JRE0uTEFCLkVORy5CUlEuUkVESEFULkNPTTEeMBwG...upSCr/8sWoLTmDxcmiE0DhXfCqatdLBBN2AoZi0dx3Md07Pn+8wrmYW+ZXfheMZlrVaTg+bh
fBqx4kuL22dynWP46b1gCKbBu4a7vYZ3tsi3ZMJW9QKauk' = 0x7f3316d758c8>(b'0\x82\x04\xa00\x82\x03\x88\xa0\x03\x02\x01\x02\x02\x01\x120\r\x06\t*\x86H\x86\xf7\r\x01\x01\x0b\x05\x000Q1/0-\x06\x0...x
e6W~\x17\x8cfZ\xd5i8>n\x17\xc1\xab\x1e$\xb8\xbd\xb6w)\xd6?\x8e\x9b\xd6\x00\x8al\x1b\xb8k\xbb\xd8g{l\x8bvL%oP)\xab\xa4')
E+where =
base64.b64encode
test_xmlrpc/test_cert_plugin.py:161: AssertionError
___ test_cert.test_0004_service_find
___
self = 0x7f330b069080>
def test_0004_service_find(self):
"""
Verify that service-find has the right certificate using
service-find.
"""
global cert
# Assume there is only one service
res = api.Command['service_find'](self.service_princ)['result']
> assert base64.b64encode(res[0]['usercertificate'][0]) == cert
E assert
b'MIIEoDCCA4igAwIBAgIBEjANBgkqhkiG9w0BAQsFADBRMS8wLQYDVQQKDCZET00tMDEyLkFCQy5JRE0uTEFCLkVORy5CUlEuU
Re: [Freeipa-devel] Another batch of Python 3 patches
On 05/03/2016 04:31 PM, Martin Basti wrote: > > > On 03.05.2016 15:52, Petr Viktorin wrote: >> On 05/03/2016 03:02 PM, Petr Spacek wrote: >>> On 2.5.2016 18:02, Martin Basti wrote: On 29.04.2016 19:46, Petr Viktorin wrote: > 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. >> 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.) > It is okay to send it in a next patch :) > ACK on this patch then Do you plan to use in py2 ? from__future__importabsolute_import >> 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. > Ok, just FYI pylint has check that prevents forgetting this import > (disabled in IPA) > 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 :) >>> Functional ACK, I did not find any breakage (when combined with other >>> Py3 >>> patches). >>> >> > 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 > ipatests.test_xmlrpc.testcert import get_testcert >xmlrpc/testcert.py:34: in > 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 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. -- Petr Viktorin -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Another batch of Python 3 patches
On 03.05.2016 15:52, Petr Viktorin wrote: On 05/03/2016 03:02 PM, Petr Spacek wrote: On 2.5.2016 18:02, Martin Basti wrote: On 29.04.2016 19:46, Petr Viktorin wrote: 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. 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.) It is okay to send it in a next patch :) ACK on this patch then Do you plan to use in py2 ? from__future__importabsolute_import 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. Ok, just FYI pylint has check that prevents forgetting this import (disabled in IPA) 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 :) Functional ACK, I did not find any breakage (when combined with other Py3 patches). 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 ipatests.test_xmlrpc.testcert import get_testcert xmlrpc/testcert.py:34: in 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 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Another batch of Python 3 patches
On 05/03/2016 03:02 PM, Petr Spacek wrote: > On 2.5.2016 18:02, Martin Basti wrote: >> >> >> On 29.04.2016 19:46, Petr Viktorin wrote: >>> 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. 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.) >> Do you plan to use in py2 ? >> from__future__importabsolute_import 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. >> >> 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 :) > > Functional ACK, I did not find any breakage (when combined with other Py3 > patches). > -- Petr Viktorin -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Another batch of Python 3 patches
On 2.5.2016 18:02, Martin Basti wrote: > > > On 29.04.2016 19:46, Petr Viktorin wrote: >> 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. > > Do you plan to use in py2 ? > from__future__importabsolute_import > > 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 :) Functional ACK, I did not find any breakage (when combined with other Py3 patches). -- Petr^2 Spacek -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] Another batch of Python 3 patches
On 29.04.2016 19:46, Petr Viktorin wrote: 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. Do you plan to use in py2 ? from__future__importabsolute_import 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 :) Martin^2 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] Another batch of Python 3 patches
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. -- Petr Viktorin From 974ff8d326e13b3121681366a2dea6503b025d77 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 29 Apr 2016 13:18:18 +0200 Subject: [PATCH] test_xmlrpc: Use absolute imports In Python 3, a module from the current package can be imported either with the absolute name or by using an explicit relative import. Part of the work for https://fedorahosted.org/freeipa/ticket/4985 --- ipatests/test_xmlrpc/test_add_remove_cert_cmd.py | 10 +- ipatests/test_xmlrpc/test_user_plugin.py | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ipatests/test_xmlrpc/test_add_remove_cert_cmd.py b/ipatests/test_xmlrpc/test_add_remove_cert_cmd.py index 122921d79f788e2c01171652ae4246e6b1717c70..0810182b35dee2e8e14466e1f454903d27e31899 100644 --- a/ipatests/test_xmlrpc/test_add_remove_cert_cmd.py +++ b/ipatests/test_xmlrpc/test_add_remove_cert_cmd.py @@ -4,13 +4,13 @@ import base64 +import pytest + from ipalib import api, errors - -from ipatests.util import assert_deepequal, raises -from xmlrpc_test import XMLRPC_test from ipapython.dn import DN -from testcert import get_testcert -import pytest +from ipatests.util import assert_deepequal, raises +from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test +from ipatests.test_xmlrpc.testcert import get_testcert @pytest.mark.tier1 diff --git a/ipatests/test_xmlrpc/test_user_plugin.py b/ipatests/test_xmlrpc/test_user_plugin.py index a70b90a994a243ea98a4d09ceec9a6b0aca49545..7c6d2ee531ed6b2945f6253861090a177e05713d 100644 --- a/ipatests/test_xmlrpc/test_user_plugin.py +++ b/ipatests/test_xmlrpc/test_user_plugin.py @@ -33,7 +33,7 @@ from ipalib import api, errors from ipatests.test_xmlrpc import objectclasses from ipatests.util import ( assert_deepequal, assert_equal, assert_not_equal, raises) -from xmlrpc_test import ( +from ipatests.test_xmlrpc.xmlrpc_test import ( XMLRPC_test, fuzzy_digits, fuzzy_uuid, fuzzy_password, fuzzy_string, fuzzy_dergeneralizedtime, add_sid, add_oc, raises_exact) from ipapython.dn import DN -- 2.5.5 From e85e876f1af99d01a8e2a7137ca6575185ad860c Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 29 Apr 2016 14:10:28 +0200 Subject: [PATCH] xmlrpc_test: Rename exception instance before working with it Python 3 unsets the exception variable at the end of an "except" block to prevent reference cycles and speed up garbage collection. Store the exception under a different name in order to use it later. Part of the work for https://fedorahosted.org/freeipa/ticket/4985 --- ipatests/test_xmlrpc/xmlrpc_test.py | 17 + 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/ipatests/test_xmlrpc/xmlrpc_test.py b/ipatests/test_xmlrpc/xmlrpc_test.py index 33088934199a2b9706f3953da1233b09c7e1315a..4052ab91868b0f0e2400b6533e5adba3fe72200d 100644 --- a/ipatests/test_xmlrpc/xmlrpc_test.py +++ b/ipatests/test_xmlrpc/xmlrpc_test.py @@ -330,14 +330,15 @@ class Declarative(XMLRPC_test): try: output = api.Command[cmd](*args, **options) except Exception as e: -pass +exception = e else: raise AssertionError( EXPECTED % (cmd, name, args, options, output) ) -if not isinstance(e, klass): +if not isinstance(exception, klass): raise AssertionError( -UNEXPECTED % (cmd, name, args, options, e.__class__.__name__, e) +UNEXPECTED % (cmd, name, args, options, + exception.__class__.__name__, exception) ) # FIXME: the XML-RPC transport doesn't allow us to return structured # information through the exception, so we can't test the kw on the @@ -345,21 +346,21 @@ class Declarative(XMLRPC_test): # transport, the exception is a free-form data structure (dict). # For now just compare the strings # pylint: disable=no-member -assert_deepequal(expected.strerror, e.strerror) +assert_deepequal(expected.strerror, exception.strerror) # pylint: enable=no-member def check_callable(self, nice, cmd, args, options, expected): name = expected.__class__.__name__ output = dict() -e = None +exception = None try: output = api.Command[cmd](*args, **options) except Exception as e: -pass -if not expected(e, output): +exception = e +if not expected(exception, output): raise AssertionError( UNEXPECTED % (cmd, name, args, options, - type(e).__name__, e) + type(exception).__name__, exception) ) def che
