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