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 OS-dependent error in upload_image. #154

Merged
merged 1 commit into from Jan 8, 2013
Merged

Conversation

Damgaard
Copy link
Contributor

@Damgaard Damgaard commented Jan 8, 2013

Some operation systems, such as windows, treat binary and text files
differently. For them opening an image with 'r' and not 'rb' would
cause the cryptic ClientException 'image is not a valid image'.
The same would also be true if the mode was some varient of 'w'
irrespective of OS.

Also, the old white-square file was a copy of the invalid file not white-square.jpg, which caused test_jpg_no_extension to fail for the minimum size requirement.

@bboe
Copy link
Member

bboe commented Jan 8, 2013

You're right. In that case, we might as well just require the user to pass a path to the file, rather than the file itself since it already requires the file to be saved to disk (StringIO doesn't work). I really don't like forcing opening in binary mode to occur when it isn't necessary on many systems. Are you up for making that change?

@Damgaard
Copy link
Contributor Author

Damgaard commented Jan 8, 2013

Sure. I'll make that change right away.

@bboe
Copy link
Member

bboe commented Jan 8, 2013

And the white-square file isn't broken. It's a symbolic link. Odds are if you're using Windows you probably need to configure something to handle the link specially.

@bboe
Copy link
Member

bboe commented Jan 8, 2013

Sure. I'll make that change right away.

Great. No need to test that the file exists. If it doesn't let python raise the IOException.

@@ -755,7 +758,8 @@ def upload_image(self, subreddit, image, name=None, header=False):
elif first_bytes.startswith(PNG_HEADER):
image_type = 'png'
else:
raise errors.ClientException('`image` is not a valid image')
Copy link
Member

Choose a reason for hiding this comment

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

Please word this:

`image` must be either a jpg or png

This fixes problems coming from opening the image file in incorrect mode.
@bboe bboe merged commit 7d044c3 into praw-dev:master Jan 8, 2013
@bboe
Copy link
Member

bboe commented Jan 8, 2013

Merged, thanks for the prompt fix.

@Damgaard Damgaard deleted the image_fix branch January 8, 2013 10:31
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.

None yet

2 participants