[Freeipa-devel] [PATCHES] 0761-0769 More Python3 fixes

[Jan Cholasta]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-545-test_ipagetkeytab-fix-missing-import.patch
Type: text/x-patch
Size: 927 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160217/19a8cb91/attachment.bin>