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