-
Notifications
You must be signed in to change notification settings - Fork 81
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
Adds more allowed mime types #1659
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks ok, Do we need to add a file size check to django-buckets
as per the conversation in the issue? Is there a reason we can't generate thumbnails for tif
images.. I'm thinking of Kesan's import requirements?
We could, but it should be a separate ticket.
We can create the thumbnail for a TIF, but it's not being displayed in some browsers. Instead, you'll see a placeholder that looks like the image wasn't loaded. That's why we excluded it for the time being, so we show the TIF icon by default. One possible solution is to convert the TIF into a PNG and create the thumbnail from that PNG. I think it's best to hold this off for when we are working moving file processing to an async worker. |
@oliverroick sounds reasonable. |
I don't think we should expand our supported mime types without first implementing the file size limit, particularly when the new list includes a lot of large file formats (video, PowerPoint/Keynote/presentation, etc). Any large files we allow to be uploaded now, we'll have to support and serve for the life of the platform. S3 has a max upload of 5GB in a single PUT -- that's a scary default limit for our contexts. |
@amplifi I agree that we should address file-size limits soon (I'm impartial about the ordering of features). Doesn't seem like too-lofty of a task either, a quick google search suggests that we could specify the EDIT: Reading over the previous comments, maybe it was already obvious as to how we could do that. 😬 |
Thanks @amplifi. I agree with you that we will need to establish a file size limit, but I do not think that we should stop the support the additional mime types. We are currently quite limited in the resource types that we can handle and partners are requesting this. |
Ok, I'll go ahead and implement this into django-buckets. I suggest to hold off this PR then until that's done. |
Thanks, @oliverroick. I'll wait on the django-buckets update to merge this one. |
b147b27
to
e467ac9
Compare
Proposed changes in this pull request
When should this PR be merged
When ready.
Risks
None
Follow-up actions
None.
Checklist (for reviewing)
General
migration
label if a new migration is added.Functionality
Code
Tests
Security
Documentation