On (12/02/15 12:22), Petr Viktorin wrote:
On 02/10/2015 11:36 PM, Lukas Slebodnik wrote:
>On (02/02/15 13:32), Petr Viktorin wrote:
>>On 02/02/2015 09:08 AM, Lukas Slebodnik wrote:
>>[...]
>>>>>Patch 1/5: Note that even under Python 2, `except ValueError,
KeyError:`
>>>>>would not work (it catches ValueError, and stores the exception
instance
>>>>>under the name KeyError); please test these changes well.
>>>>>
>>>>It means that in python2 KeyError was not handled at all.
>>>>
>>>>python2 documentation says:
>>>>"If an exception occurs which does not match the exception named in
the except
>>>>clause, it is passed on to outer try statements; if no handler is found,
it is
>>>>an unhandled exception and execution stops with a message as shown
above."
>>>>
>>>>In my patch, you can see that different exception was thrown
(ParsingError) in
>>>>handler. I'm not sure hot to test it therefore you are in CC ;-)
>>>>Shall I remove KeyError?
>>
>>Yes, remove KeyError. Looking at the history, since 2009 the code worked
>>without catching KeyError (even if that wasn't the intention). I see no point
>>in adding it now.
>>
>Removed
>
>>Otherwise LGTM
>>
>>>>
>>>>>Patch 4/5: you can use io.StringIO in both 2.6+ and 3.x+
>>>>>
>>>>Yes, but...
>>>>Traceback (most recent call last):
>>>> File "../src/sbus/sbus_codegen", line 931, in <module>
>>>> main()
>>>> File "../src/sbus/sbus_codegen", line 919, in main
>>>> generate_source(parser.parsed_interfaces, filename, options.include)
>>>> File "../src/sbus/sbus_codegen", line 600, in generate_source
>>>> out("/* The following definitions are auto-generated from %s
*/", basename)
>>>> File "../src/sbus/sbus_codegen", line 217, in out
>>>> sys.stdout.write(str)
>>>>TypeError: unicode argument expected, got 'str'
>>>>
>>>>So I'm find with such solution for StringIO. The sbus_codegen is
python script
>>>>which generate C source file. I do not expect any unicode characters.
>>>>But I'm not opposed better version.
>>
>>I see.
>>People could expect "io" to be the io module, so you could write
>>
>> if sys.version_info[0] > 2:
>> from io import StringIO
>> else:
>> from StringIO import StringIO
>> ...
>> sys.stdout = buf = StringIO()
>>
>Fixed
>
>>... but at this point that's just nitpicking.
>>
>>>>
>>>>Thank you very much for review.
>>>>I squashed some small patches.
>>>>
>>>>LS
>>>
>>>I realized there were hardcoded python2 on some places. The 3rd patch should
>>>fix it. I used "env pyhton" for internal python scripts and for
tests which are
>>>not installed.
>>>
>>>I had a discussion with Pavel B. and we agreed
>>>that patch "sbus_codegen: Port to python3" needn't be push to
upstream yet.
>>>It would break his other patches. So current version would be used mostly by
>>>downstream.
>>
>Thank you very much for review.
>
>Updated version is attached.
LGTM
Here is a link to ci result
http://sssd-ci.duckdns.org/logs/job/7/32/summary.html.
I think we should push these patches also to stable branch sssd-1.12.
So package maintainers can decide wheter to build with python2 or python3.
The other patches on the list which allow to build bindings for python2 and
python3 are quite invasive and changes some behavoiur therefore are not
intended to push to stable branch.
These patches depends on Bohuslav's patch
"Python3 support in SSSD" 341a00311680a440d7f979f06c34c70d86c9367a
"BUILD: Include python-test.py in the tarball"
51d65c4ad15c2cc23f38fa09dd6efeb15e4f3e86
BTW We agreed with Pavel (pbrezina) to do not push patch sbus_codegen patch to
master because he did some changes in that file. So I will prepare next version
afther his patches will be pushed.
LS