search

neutronX / django-markdownx

Comprehensive Markdown plugin built for Django
https://neutronx.github.io/django-markdownx/
Other
863 stars 153 forks source link

Reset image stream position after using tell #105

Closed ezarowny closed 6 years ago

ezarowny commented 6 years ago

For uploads, I noticed that using django-markdownx with something like django-storages doesn't work because the image stream position is set to the end of the steam when the InMemoryUploadedFile is instantiated. Simply resetting the image stream position back to the beginning should do the trick.

In the future, when you guys drop Python 2 support, something like getbuffer or getvalue could be used to get the length of the buffer instead of using tell.

xenatisch commented 6 years ago

Nice catch. You tested this in different scenarios? It's not breaking any of the forthcoming features?

ezarowny commented 6 years ago

I'll run down some more test cases. I've mostly been testing this against Google Cloud Storage but it shouldn't really make much of a difference.

xenatisch commented 6 years ago

Thanks again.

It's not the storage I'm worried about; it's the forthcoming attributes that are applied. Just test it against PNG, JPEG, and SVG files. If they all work, then we're good to go.

ezarowny commented 6 years ago

What would you like me to look for besides the upload working and displaying inline in admin? The file types are correct in storage (I'm testing locally to rule out any Google weirdness).

JPEG, PNG and SVG all seem to work for me.

xenatisch commented 6 years ago

Nothing... We're all good if all the uploads are working.

Is Google storage really that weird? I keep hearing about it!

xenatisch commented 6 years ago

In the future, when you guys drop Python 2 support, something like getbuffer or getvalue could be used to get the length of the buffer instead of using tell.

I know - and how I wish to see the back of Python 2!

Would you mind adding this recommendation to #60 - just so we won't forget! Or better yet, just change it yourself and send a pull request our way. We are dropping Python 2 support in the next version, so...

ezarowny commented 6 years ago

Eh, I wouldn't say that GCS is all that weird. You just can't necessarily treat it like a normal file stream. At least that's the case for the Google-provided Python library.

👍 for the Python 3 love