Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix default encoding issue with python 2.6 #40

Merged
merged 2 commits into from Aug 14, 2013
Merged

Fix default encoding issue with python 2.6 #40

merged 2 commits into from Aug 14, 2013

Conversation

dhellmann
Copy link

This change addresses issue #38: "fix unicode handling issues".

The issue was originally reported against neutron client
(https://bugs.launchpad.net/python-neutronclient/+bug/1189112) but
was tracked down to the fact that python 2.6 does not set the default
encoding for sys.stdout properly. A change to python 2.7 fixes the
problem there and later (http://hg.python.org/cpython/rev/e60ef17561dc/),
but since cliff supports python 2.6 it needs to handle the case
explicitly.

Change-Id: Id06507d78c7c82b25f39366ea4a6dfa4ef3a3a97

@iamhappg
Copy link

iamhappg commented Aug 2, 2013

By default (python 2.6), sys.stdout has the attribute "encoding", so the logic flow always goes to "elif getattr(default, 'encoding', None):
stream = default "
and it never goes to "stream = wrapper_factory(encoding)(default) "
To fix the bug, we need to make sure the logic flow goes to "stream = wrapper_factory(encoding)(default) ", when we use sys.stdout.

and I think:
1.1
elif getattr(default, 'encoding', None):
stream = default "
setattr(self, attr_name, stream)
1.2
self.stdout = sys.stdout
The meanings of 1.1 and 1.2 are same for this bug. It does not make sense.

@dhellmann
Copy link
Author

If sys.stdout already has an encoding value, then it cannot just be wrapped with a writer from codecs. We would have to unwrap it, and rewrap it.

Is the problem that sys.stdout's encoding is wrong?

@iamhappg
Copy link

iamhappg commented Aug 5, 2013

sys.stdout's encoding is right. We just need to make sure sys.stdout.write() use sys.stdout.encoding instead of sys.getdefaultencoding() in python 2.6 . "stream = wrapper_factory(encoding)(default)" can do that in this fix, But the logic flow can not goes to it, the logic flow always goes to "stream = default", then sys.stdout.write() still use sys.getdefaultencoding() in python 2.6.
.
sys.stdout.write() use sys.getdefaultencoding(), instead of sys.stdout.encoding in python 2.6,
By default, sys.getdefaultencoding() is "ascii''.
So we will see the error:
'ascii' codec can't encode characters in position 561-569: ordinal not in range(128)

Please see : http://stackoverflow.com/questions/8016236/python-unicode-handling-differences-between-print-and-sys-stdout-write : "The documentation states that when unicode strings are written to a file, they should be converted to byte strings using file.encoding. But this was not being honoured by sys.stdout, which instead was using the default unicode encoding. This is usually set to "ascii" by the site module."

This change addresses issue #38: "fix unicode handling issues".

The issue was originally reported against neutron client
(https://bugs.launchpad.net/python-neutronclient/+bug/1189112) but
was tracked down to the fact that python 2.6 does not set the default
encoding for sys.stdout properly. A change to python 2.7 fixes the
problem there and later (http://hg.python.org/cpython/rev/e60ef17561dc/),
but since cliff supports python 2.6 it needs to handle the case
explicitly.

Change-Id: Id06507d78c7c82b25f39366ea4a6dfa4ef3a3a97
@dhellmann
Copy link
Author

OK, I get it now. The latest version of the change should do what you describe. I have also created a plugin for the demo app to verify that it works as expected.

Change-Id: I3bbd270a2a0ed6eee6eb12a326f86aaad84f2aad
@iamhappg
Copy link

It works in my environment. Thanks for your update.

dhellmann added a commit that referenced this pull request Aug 14, 2013
Fix default encoding issue with python 2.6
@dhellmann dhellmann merged commit aed63e0 into dreamhost:master Aug 14, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants