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

[gcloud] Add support for MIME type #320

Merged
merged 1 commit into from
Jul 2, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ setuptools*
__pycache__
.coverage
.cache
.idea

.idea/
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ By order of apparition, thanks:
* Eirik Martiniussen Sylliaas (Google Cloud Storage native support)
* Jody McIntyre (Google Cloud Storage native support)
* Stanislav Kaledin (Bug fixes in SFTPStorage)
* Filip Vavera (Google Cloud MIME types support)

Extra thanks to Marty for adding this in Django,
you can buy his very interesting book (Pro Django).
7 changes: 5 additions & 2 deletions storages/backends/gcloud.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import mimetypes
from tempfile import SpooledTemporaryFile

from django.core.exceptions import ImproperlyConfigured
Expand All @@ -21,6 +22,7 @@
class GoogleCloudFile(File):
def __init__(self, name, mode, storage):
self.name = name
self.mime_type = mimetypes.guess_type(name)[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

Since guess_type isn't 100% reliable, I think we need to allow the user to override the type here if needed. Can you add that?

Copy link
Contributor Author

@Sgiath Sgiath Jun 27, 2017

Choose a reason for hiding this comment

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

When the user should override it? Globally at the Django settings (set the desired MIME type for specific file extension)? Something like:

GS_MIME_TYPE = {
    '.css': 'text/css',
    '.mp4': 'video/mp4',
}

I didn't add it because MIME types are not THAT important. They can be edited manually in Google Cloud Console if really needed and almost everything should work also without them. So I didn't want to add complexity with another settings option.
But if you thinks it is needed I can definitely add this to the code.

Copy link
Contributor

Choose a reason for hiding this comment

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

Now that I've looked more closely, I don't see a good place to add this. Django's storage API doesn't have any support for MIME types as far as I can tell, so we're well within our rights to set whatever we want when saving a file to the storage provider.

self._mode = mode
self._storage = storage
self.blob = storage.bucket.get_blob(name)
Expand Down Expand Up @@ -70,7 +72,7 @@ def close(self):
if self._file is not None:
if self._is_dirty:
self.file.seek(0)
self.blob.upload_from_file(self.file)
self.blob.upload_from_file(self.file, content_type=self.mime_type)
self._file.close()
self._file = None

Expand Down Expand Up @@ -154,7 +156,8 @@ def _save(self, name, content):
content.name = cleaned_name
encoded_name = self._encode_name(name)
file = GoogleCloudFile(encoded_name, 'rw', self)
file.blob.upload_from_file(content, size=content.size)
file.blob.upload_from_file(content, size=content.size,
content_type=file.mime_type)
return cleaned_name

def delete(self, name):
Expand Down
8 changes: 5 additions & 3 deletions tests/test_gcloud.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import mock

import datetime
import mimetypes

from django.core.files.base import ContentFile
from django.test import TestCase
Expand Down Expand Up @@ -90,7 +91,8 @@ def test_open_write(self, MockBlob):
# File data is not actually written until close(), so do that.
f.close()

MockBlob().upload_from_file.assert_called_with(tmpfile)
MockBlob().upload_from_file.assert_called_with(
tmpfile, content_type=mimetypes.guess_type(self.filename)[0])

def test_save(self):
data = 'This is some test content.'
Expand All @@ -100,7 +102,7 @@ def test_save(self):

self.storage._client.get_bucket.assert_called_with(self.bucket_name)
self.storage._bucket.get_blob().upload_from_file.assert_called_with(
content, size=len(data))
content, size=len(data), content_type=mimetypes.guess_type(self.filename)[0])

def test_save2(self):
data = 'This is some test ủⓝï℅ⅆℇ content.'
Expand All @@ -111,7 +113,7 @@ def test_save2(self):

self.storage._client.get_bucket.assert_called_with(self.bucket_name)
self.storage._bucket.get_blob().upload_from_file.assert_called_with(
content, size=len(data))
content, size=len(data), content_type=mimetypes.guess_type(filename)[0])

def test_delete(self):
self.storage.delete(self.filename)
Expand Down