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

Google: Only verify bucket if auto_create is True #718

Merged
merged 1 commit into from Sep 8, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 16 additions & 17 deletions storages/backends/gcloud.py
Expand Up @@ -15,9 +15,8 @@
)

try:
from google.cloud.storage.client import Client
from google.cloud.storage.blob import Blob
from google.cloud.exceptions import NotFound
from google.cloud.storage import Blob, Client
from google.cloud.exceptions import Conflict, NotFound
except ImportError:
raise ImproperlyConfigured("Could not load Google Cloud Storage bindings.\n"
"See https://github.com/GoogleCloudPlatform/gcloud-python")
Expand Down Expand Up @@ -129,19 +128,19 @@ def bucket(self):

def _get_or_create_bucket(self, name):
"""
Retrieves a bucket if it exists, otherwise creates it.
Returns bucket. If auto_create_bucket is True, creates bucket if it
doesn't exist.
"""
try:
return self.client.get_bucket(name)
except NotFound:
if self.auto_create_bucket:
bucket = self.client.create_bucket(name)
bucket.acl.save_predefined(self.auto_create_acl)
return bucket
raise ImproperlyConfigured("Bucket %s does not exist. Buckets "
"can be automatically created by "
"setting GS_AUTO_CREATE_BUCKET to "
"``True``." % name)
bucket = self.client.bucket(name)
if self.auto_create_bucket:
try:
new_bucket = self.client.create_bucket(name)
new_bucket.acl.save_predefined(self.auto_create_acl)
return new_bucket
except Conflict:
# Bucket already exists
pass
return bucket

def _normalize_name(self, name):
"""
Expand Down Expand Up @@ -191,9 +190,9 @@ def delete(self, name):
def exists(self, name):
if not name: # root element aka the bucket
try:
self.bucket
self.client.get_bucket(self.bucket)
return True
except ImproperlyConfigured:
except NotFound:
return False

name = self._normalize_name(clean_name(name))
Expand Down
28 changes: 19 additions & 9 deletions tests/test_gcloud.py
Expand Up @@ -12,7 +12,7 @@
from django.core.files.base import ContentFile
from django.test import TestCase
from django.utils import timezone
from google.cloud.exceptions import NotFound
from google.cloud.exceptions import Conflict, NotFound
from google.cloud.storage.blob import Blob

from storages.backends import gcloud
Expand Down Expand Up @@ -41,7 +41,7 @@ def test_open_read(self):
data = b'This is some test read data.'

f = self.storage.open(self.filename)
self.storage._client.get_bucket.assert_called_with(self.bucket_name)
self.storage._client.bucket.assert_called_with(self.bucket_name)
self.storage._bucket.get_blob.assert_called_with(self.filename)

f.blob.download_to_file = lambda tmpfile: tmpfile.write(data)
Expand All @@ -52,7 +52,7 @@ def test_open_read_num_bytes(self):
num_bytes = 10

f = self.storage.open(self.filename)
self.storage._client.get_bucket.assert_called_with(self.bucket_name)
self.storage._client.bucket.assert_called_with(self.bucket_name)
self.storage._bucket.get_blob.assert_called_with(self.filename)

f.blob.download_to_file = lambda tmpfile: tmpfile.write(data)
Expand Down Expand Up @@ -102,7 +102,7 @@ def test_save(self):

self.storage.save(self.filename, content)

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

Expand All @@ -113,7 +113,7 @@ def test_save2(self):

self.storage.save(filename, content)

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

Expand All @@ -129,15 +129,15 @@ def test_save_with_default_acl(self):

self.storage.save(filename, content)

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

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

self.storage._client.get_bucket.assert_called_with(self.bucket_name)
self.storage._client.bucket.assert_called_with(self.bucket_name)
self.storage._bucket.delete_blob.assert_called_with(self.filename)

def test_exists(self):
Expand All @@ -160,12 +160,22 @@ def test_exists_bucket(self):
# exists('') should return True if the bucket exists
self.assertTrue(self.storage.exists(''))

def test_exists_no_bucket_auto_create(self):
# exists('') should return true when auto_create_bucket is configured
# and bucket already exists
# exists('') should automatically create the bucket if
# auto_create_bucket is configured
self.storage.auto_create_bucket = True
self.storage._client = mock.MagicMock()
self.storage._client.create_bucket.side_effect = Conflict('dang')

self.assertTrue(self.storage.exists(''))

def test_exists_bucket_auto_create(self):
# exists('') should automatically create the bucket if
# auto_create_bucket is configured
self.storage.auto_create_bucket = True
self.storage._client = mock.MagicMock()
self.storage._client.get_bucket.side_effect = NotFound('dang')

self.assertTrue(self.storage.exists(''))
self.storage._client.create_bucket.assert_called_with(self.bucket_name)
Expand Down Expand Up @@ -383,7 +393,7 @@ def test_cache_control(self):
self.storage.cache_control = cache_control
self.storage.save(filename, content)

bucket = self.storage.client.get_bucket(self.bucket_name)
bucket = self.storage.client.bucket(self.bucket_name)
blob = bucket.get_blob(filename)
self.assertEqual(blob.cache_control, cache_control)

Expand Down