On 17.2.2016 10:43, Jan Cholasta wrote:
On 9.2.2016 18:02, Petr Viktorin wrote:
On 01/29/2016 09:42 AM, Jan Cholasta wrote:
On 29.1.2016 09:25, Jan Cholasta wrote:
Hi,

On 27.1.2016 18:38, Petr Viktorin wrote:
Hello,

Here is a mixed bag of Python 3 fixes.
They fix some tests, and they should enable you to use `python3
/usr/bin/ipa`.

Patch 761:

1) The "invalid 'my_number': " bit comes from IPA itself, shouldn't we
check at least that?

Fixed

Patch 762:

1) We should handle UnicodeError here as well, in addition to
TypeError:

                   if k.lower() == 'negotiate':
                       try:
-                        token = base64.b64decode(v)
+                        token = base64.b64decode(v.encode('ascii'))
                           break
                       # b64decode raises TypeError on invalid input
                       except TypeError:

Fixed

2) I would prefer if the encoding was specified explicitly here:

+            response =
json_decode_binary(json.loads(response.decode()))

Fixed

Patch 763:

1)

+                    altname = altname

Fixed

2) Nitpick, but could you please:

-        if isinstance(name_or_oid, unicode):
-            name_or_oid = name_or_oid.encode('utf-8')
+        if six.PY2:
+            if isinstance(name_or_oid, unicode):
+                name_or_oid = name_or_oid.encode('utf-8')

This way it's more visible that this is a py2-only thing.

Sure

Patch 764: LGTM


Patch 765:

1)

+import tempfile
+import tempfile

Fixed.

Patch 766-767: LGTM


Patch 768:

1) Only binascii.Error should be handled in int_to_bytes, the
try-except
block is there just to handle odd-length strings.

That's there for Python 2, where unhexlify raises TypeError.

2) I think you can just remove the library_path.encode(), it's there
because the original C code did the same thing, but don't think it's
necessary.

OK

Patch 769: LGTM

Also, could you please reference
<https://fedorahosted.org/freeipa/ticket/5638> in the patches?

Sure.

Thanks for the review! Updated patches attached.

Thanks, ACK.

Pushed to:
master: d1252cfb8ef613e290a23e0e9cabd9d135a129ca
ipa-4-3: c8c2a6d33815b478c1d6ec402eaa2f56d0fe61b7

I screwed up. The attached patch fixes that.

Pushed under the one-liner rule to ipa-4-3: 5ff41fa3091235aaf48e855b0c3ede0154a58dfc

--
Jan Cholasta
From 554d6d39d578375dc5990ea5b053736454a607e1 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Wed, 17 Feb 2016 10:49:45 +0100
Subject: [PATCH] test_ipagetkeytab: fix missing import

The import was accidentally removed in commit
9a3d5fdadd691df722b4a2ee29642a81a0035ad3.

https://fedorahosted.org/freeipa/ticket/5638
---
 ipatests/test_cmdline/test_ipagetkeytab.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ipatests/test_cmdline/test_ipagetkeytab.py b/ipatests/test_cmdline/test_ipagetkeytab.py
index 3c91f37..6153b9b 100644
--- a/ipatests/test_cmdline/test_ipagetkeytab.py
+++ b/ipatests/test_cmdline/test_ipagetkeytab.py
@@ -33,6 +33,7 @@ from ipalib import errors
 from ipapython import ipautil, ipaldap
 from ipaserver.plugins.ldap2 import ldap2
 from ipapython.dn import DN
+from ipatests.test_cmdline.cmdline import cmdline_test
 
 def use_keytab(principal, keytab):
     try:
-- 
2.5.0

-- 
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

Reply via email to