matthewwithanm / pilkit

Utilities and processors built for, and on top of PIL
BSD 3-Clause "New" or "Revised" License
196 stars 54 forks source link

adds support for remote images (S3, CloudFiles,..) #10

Closed baffolobill closed 10 years ago

baffolobill commented 10 years ago

Adds support for remote images in the same way sorl.thumbnail does (https://github.com/mariocesar/sorl-thumbnail/blob/master/sorl/thumbnail/engines/pil_engine.py#L46).

matthewwithanm commented 10 years ago

Thanks @baffolobill! Do we still need to seek to the beginning of the file?

baffolobill commented 10 years ago

No, we don't. During StringIO initialization attribute self.pos is set to 0 (http://svn.python.org/projects/stackless/trunk/Lib/StringIO.py). So it looks like for me as excess operation. But if we replace this line

buffer = StringIO(target.read())

to

buffer = StringIO()
buffer.write(target.read())

we have to do buffer.seek(0).

matthewwithanm commented 10 years ago

Alright, this is merged for 1.1.7. Thanks so much for the pull request. Sorry it took a week to get to it!

michaelshobbs commented 10 years ago

Not sure why yet but this seems to break django-imagekit and/or PIL. Probably the latter actually.

... File "/venv/local/lib/python2.7/site-packages/imagekit/specs/init.py", line 149, in generate img = open_image(self.source) File "/venv/local/lib/python2.7/site-packages/pilkit/utils.py", line 22, in open_image return Image.open(buffer) File "/venv/local/lib/python2.7/site-packages/PIL/Image.py", line 1980, in open raise IOError("cannot identify image file")

matthewwithanm commented 10 years ago

@michaelshobbs Thanks for the report.

@baffolobill I've had to revert this change but if you want to submit another PR that doesn't make PIL mad, I'll merge that.

Sorry all for the confusion.

baffolobill commented 10 years ago

@michaelshobbs It seems the problem in PIL. Could you provide output for a command:

pip freeze | grep -E '(Pillow|PIL)'

(explanation here http://stackoverflow.com/a/18875035/1492286)

michaelshobbs commented 10 years ago

just PIL and pilkit.

pip freeze | grep -E '(Pillow|PIL)' PIL==1.1.7 pilkit==1.1.7

baffolobill commented 10 years ago

@michaelshobbs was it remote or local image? If remote, post a link to the image, if it's possible.

michaelshobbs commented 10 years ago

Just a local test image. Works perfectly when I downgrade to pilkit==1.1.6

baffolobill commented 10 years ago

@matthewwithanm I think the best (and stupid) solution for now is https://github.com/baffolobill/pilkit/commit/89432b5e9796a4e9f8fc1f76c1364e8336642618

@michaelshobbs Could you try this solution:

pip install -U git+https://github.com/baffolobill/pilkit.git#89432b5e9796a4e9f8fc1f76c1364e8336642618)

and report about bugs, please?

michaelshobbs commented 10 years ago

That seems to work.

matthewwithanm commented 10 years ago

Hm, I'm not so sure about this. The error is caused by Image.open so, in the case of a failure, we've done an unnecessary read.

matthewwithanm commented 10 years ago

@michaelshobbs I'm trying to reproduce but not having any luck. Can you try the following and share the output?

>>> from PIL import Image
>>> from cStringIO import StringIO
>>> print Image.__file__
>>> Image.open(StringIO(open('/path/to/image.jpg').read()))
michaelshobbs commented 10 years ago

pilkit==1.1.6

pip freeze:

PIL==1.1.7
pilkit==1.1.6
wsgiref==0.1.2

output:

Python 2.7.6 (default, Mar 13 2014, 10:34:57)
[GCC 4.2.1 Compatible Apple LLVM 5.1 (clang-503.0.38)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from PIL import Image
>>> from cStringIO import StringIO
>>> print Image.__file__
/Users/mhobbs/.virtualenvs/test_env/lib/python2.7/site-packages/PIL/Image.pyc
>>> Image.open(StringIO(open('./test.jpg').read()))
<PIL.JpegImagePlugin.JpegImageFile image mode=RGB size=350x503 at 0x102128488>

pilkit==1.1.7

pip freeze:

PIL==1.1.7
pilkit==1.1.7
wsgiref==0.1.2

output:

[GCC 4.2.1 Compatible Apple LLVM 5.1 (clang-503.0.38)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from PIL import Image
>>> from cStringIO import StringIO
>>> print Image.__file__
/Users/mhobbs/.virtualenvs/test_env/lib/python2.7/site-packages/PIL/Image.pyc
>>> Image.open(StringIO(open('./test.jpg').read()))
<PIL.JpegImagePlugin.JpegImageFile image mode=RGB size=350x503 at 0x10BACC488>
matthewwithanm commented 10 years ago

@michaelshobbs Hope you don't mind; I edited your last comment to use fenced code blocks.

I'm surprised that there wasn't any error for that. I'm going to have to dig into this more sometime to figure out what exactly is causing the issue. I can't reopen this issue (since I merged the PR), so I've opened #13. If either of you figure out the direct cause of the error, let me know.

michaelshobbs commented 10 years ago

Sounds good. Markdown and I have a hate-hate relationship.

I'll try to do some more debugging today.

matthewwithanm commented 10 years ago

:smile: Thanks!