On (29/01/15 13:09), Petr Viktorin wrote:
> On 01/29/2015 01:05 PM, Bohuslav Kabrda wrote:
>> ----- Original Message -----
>>> ehlo,
>>>
>>> some parts of sssd was not properly ported to python3.
>>> I know there were changes related to unicode, string and bytes.
>>> I am not sure whether my patches for read and open are correct,
>>> especially patch "SSSDConfig: os.write".
>>>
>>> SSSDConfig (python-sssdconfig is used by authconfig and ipa-client-install?
>>> So I don't want to break it.
>>>
>>> Please review patches or propose better version.
>>>
>>> LS
>>
>> Not sure why I missed so many things in my original patch (maybe I searched
>> just for *.py, not *.py.in ;)). Anyway:
and IIRC sbus_codegen was not in git when you sent patches.
There are lots of patches on the list that touches sbus_codegen. Please
wait with those if it conflicts.
>> - Patch 1 LGTM, I only don't understand why you need to import
"exceptions"
>> module (and why only in Python 2). Note that Python 2 documentation says
>> that "This module never needs to be imported explicitly". [1]
It might have been used due to older version of python (RHEL5),
which we do not support any more.
I removed it.
>> - Patch 2 LGMT, but I think you could just write:
>>
>> 'long': long if sys.version_info[0] == 2 else int
>>
That's nicer.
>> (but that's just nitpicking...)
>> - Patch 3: this depends. Is the object that you're writing binary or text?
>> I think what you want to do is keep "wb" mode and encode output to a
certain
>> encoding (utf8?). So this could be:
>>
>> of = open(outputfile, "wb")
>> output = self.dump(self.opts).encode("utf-8")
>> of.write(output)
Output file should be valid sssd configuration file which if text file.
I used your proposal.
>> The problem is that the "str" type in Python 2 is sort of polymorphic,
in
>> that it can behave both as binary bytestring and as unicode - but .encode()
>> works on it nonetheless. In Python 3 you have either "bytes" (binary
string)
>> or "str" (unicode text string) and you have to choose which one you
want
>> to write ("wb" mode is for "bytes", while "w" is
for "str").
>> If "self.dump(self.opts)" returns "str" in Python 3, then I
suggest you use
>> '.encode("utf-8")' on it and leave the mode "wb".
>> This should keep working in Python 2.
>> - Patch 4 LGTM
>> - Patch 5 LGTM
>>
>> Slavek
>>
>> [1]
https://docs.python.org/2/library/exceptions.html#module-exceptions
>>
>
> 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?
> 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.
Thank you very much for review.
I squashed some small patches.
LS