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

Fixed Exception clause left in Python 2 style. #96

Closed
wants to merge 1 commit into from

Conversation

fccoelho
Copy link

Fixed small bug with bytes to str conversion

This commit handle the bugs reported on issue #95

Fixed small bug with bytes to str conversion

This commit handle the bugs reported on  issue ckan#95
@@ -107,9 +107,9 @@ def main(running_with_paster=False):
if arguments['action']:
try:
for r in action(ckan, arguments):
sys.stdout.write(r)
sys.stdout.write(str(r))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this might not work. if the output is pretty json and includes non-ascii characters then we would need stdout = getattr(sys.stdout, 'buffer', sys.stdout) and no str(r) here

@fccoelho
Copy link
Author

I ran the following test here and it worked:

In [7]:  js = {'name': 'São paulo'}

In [8]: str(js)
Out[8]: "{'name': 'São paulo'}"

In [9]: sys.stdout.write(str(js))
Out[9]: {'name': 'São paulo'}21

Can you give an example where it does not?

On the other hand if I follow your suggestions this is what happens:

In [11]: stdout = getattr(sys.stdout, 'buffer', sys.stdout)

In [12]: stdout.write(js)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-12-f597dc5b97ea> in <module>()
----> 1 stdout.write(js)

TypeError: a bytes-like object is required, not 'dict'

in order to fix it I'd have to convert the object to bytes:

In [14]: stdout.write(str(js).encode())
{'name': 'São paulo'}Out[14]: 22

@rwillmer rwillmer mentioned this pull request Dec 7, 2016
@wardi
Copy link
Contributor

wardi commented Dec 7, 2016

@fccoelho It would fail in Python 2. str(u'São paulo') raises an exception.

@fccoelho
Copy link
Author

fccoelho commented Dec 7, 2016

Do you still need to support python2?

Anyway, it works as long you omit the u and use a regular string instead of an u'string'

the following works on both Python 2 and 3:

sys.stdout.write(str('São paulo'))

@wardi
Copy link
Contributor

wardi commented Dec 7, 2016

Yes, we need to support Python 2 because that's required for -c (running the api calls locally directly on a ckan config file)

It looks like action() does yield byte strings, so then this change is actually a no-op in python 2 but in Python 3 if you call str() on a byte string you'll get the repr of what's returned, which isn't valid JSON.

@timwis
Copy link

timwis commented Apr 9, 2017

Hi folks, any update on this? Just ran into the TypeError: write() argument must be str, not bytes issue as well using python3 and ckanapi v4.

@wardi
Copy link
Contributor

wardi commented Sep 13, 2017

fixed by fa920b2

@wardi wardi closed this Sep 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants