From d9d3657a7cb11d511a60d7f6f1ef91c022c7801e Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Wed, 25 Mar 2015 13:33:17 -0700 Subject: [PATCH 1/3] Adding implicit connection fallback in Bucket. --- gcloud/storage/__init__.py | 1 + gcloud/storage/batch.py | 5 ++++ gcloud/storage/bucket.py | 28 +++++++++++++++++- gcloud/storage/test_bucket.py | 55 ++++++++++++++++++++++++++++++++++- regression/storage.py | 3 +- 5 files changed, 88 insertions(+), 4 deletions(-) diff --git a/gcloud/storage/__init__.py b/gcloud/storage/__init__.py index 200d863ea544..d53861fd7b79 100644 --- a/gcloud/storage/__init__.py +++ b/gcloud/storage/__init__.py @@ -51,6 +51,7 @@ from gcloud.storage.api import get_all_buckets from gcloud.storage.api import get_bucket from gcloud.storage.api import lookup_bucket +from gcloud.storage.batch import Batch from gcloud.storage.blob import Blob from gcloud.storage.bucket import Bucket from gcloud.storage.connection import Connection diff --git a/gcloud/storage/batch.py b/gcloud/storage/batch.py index 62fdb6ac18e6..ad811d234184 100644 --- a/gcloud/storage/batch.py +++ b/gcloud/storage/batch.py @@ -155,6 +155,11 @@ def finish(self): self._responses = list(_unpack_batch_response(response, content)) return self._responses + @staticmethod + def current(): + """Return the topmost batch, or None.""" + return _BATCHES.top + def __enter__(self): _BATCHES.push(self) return self diff --git a/gcloud/storage/bucket.py b/gcloud/storage/bucket.py index 0200d766d126..8116c8d8bd0f 100644 --- a/gcloud/storage/bucket.py +++ b/gcloud/storage/bucket.py @@ -39,9 +39,11 @@ from gcloud.exceptions import NotFound from gcloud.storage._helpers import _PropertyMixin from gcloud.storage._helpers import _scalar_property +from gcloud.storage import _implicit_environ from gcloud.storage.acl import BucketACL from gcloud.storage.acl import DefaultObjectACL from gcloud.storage.iterator import Iterator +from gcloud.storage.batch import Batch from gcloud.storage.blob import Blob @@ -146,7 +148,7 @@ def connection(self): :rtype: :class:`gcloud.storage.connection.Connection` :returns: The connection to use. """ - return self._connection + return _require_connection(self._connection) @staticmethod def path_helper(bucket_name): @@ -746,3 +748,27 @@ def make_public(self, recursive=False, future=False): for blob in self: blob.acl.all().grant_read() blob.save_acl() + + +def _require_connection(connection=None): + """Infer a connection from the environment, if not passed explicitly. + + :type connection: :class:`gcloud.storage.connection.Connection` + :param connection: Optional. + + :rtype: :class:`gcloud.storage.connection.Connection` + :returns: A connection based on the current environment. + :raises: :class:`EnvironmentError` if ``connection`` is ``None``, and + cannot be inferred from the environment. + """ + # NOTE: We use current Batch directly since it inherits from Connection. + if connection is None: + connection = Batch.current() + + if connection is None: + connection = _implicit_environ.get_default_connection() + + if connection is None: + raise EnvironmentError('Connection could not be inferred.') + + return connection diff --git a/gcloud/storage/test_bucket.py b/gcloud/storage/test_bucket.py index 081344598825..bfc84765cab4 100644 --- a/gcloud/storage/test_bucket.py +++ b/gcloud/storage/test_bucket.py @@ -71,7 +71,7 @@ def _makeOne(self, *args, **kw): def test_ctor_defaults(self): bucket = self._makeOne() - self.assertEqual(bucket.connection, None) + self.assertEqual(bucket._connection, None) self.assertEqual(bucket.name, None) self.assertEqual(bucket._properties, {}) self.assertTrue(bucket._acl is None) @@ -1068,6 +1068,44 @@ def get_items_from_response(self, response): self.assertEqual(kw[1]['query_params'], {}) +class Test__require_connection(unittest2.TestCase): + + def _callFUT(self, connection=None): + from gcloud.storage.bucket import _require_connection + return _require_connection(connection=connection) + + def _monkey(self, connection): + from gcloud.storage._testing import _monkey_defaults + return _monkey_defaults(connection=connection) + + def test_implicit_unset(self): + with self._monkey(None): + with self.assertRaises(EnvironmentError): + self._callFUT() + + def test_implicit_unset_w_existing_batch(self): + CONNECTION = object() + with self._monkey(None): + with _NoCommitBatch(connection=CONNECTION): + self.assertEqual(self._callFUT(), CONNECTION) + + def test_implicit_unset_passed_explicitly(self): + CONNECTION = object() + with self._monkey(None): + self.assertTrue(self._callFUT(CONNECTION) is CONNECTION) + + def test_implicit_set(self): + IMPLICIT_CONNECTION = object() + with self._monkey(IMPLICIT_CONNECTION): + self.assertTrue(self._callFUT() is IMPLICIT_CONNECTION) + + def test_implicit_set_passed_explicitly(self): + IMPLICIT_CONNECTION = object() + CONNECTION = object() + with self._monkey(IMPLICIT_CONNECTION): + self.assertTrue(self._callFUT(CONNECTION) is CONNECTION) + + class _Connection(object): _delete_bucket = False @@ -1118,3 +1156,18 @@ class MockFile(io.StringIO): def __init__(self, name, buffer_=None): super(MockFile, self).__init__(buffer_) self.name = name + + +class _NoCommitBatch(object): + + def __init__(self, connection): + self._connection = connection + + def __enter__(self): + from gcloud.storage.batch import _BATCHES + _BATCHES.push(self._connection) + return self._connection + + def __exit__(self, *args): + from gcloud.storage.batch import _BATCHES + _BATCHES.pop() diff --git a/regression/storage.py b/regression/storage.py index 9d9a7f585514..195c5f660b7b 100644 --- a/regression/storage.py +++ b/regression/storage.py @@ -22,7 +22,6 @@ from gcloud import storage from gcloud import _helpers from gcloud.storage._helpers import _base64_md5hash -from gcloud.storage.batch import Batch HTTP = httplib2.Http() @@ -52,7 +51,7 @@ def setUp(self): self.case_buckets_to_delete = [] def tearDown(self): - with Batch() as batch: + with storage.Batch() as batch: for bucket_name in self.case_buckets_to_delete: storage.Bucket(bucket_name, connection=batch).delete() From 7ce72bafd9401b1895cad2b6ff45c13904d745af Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Wed, 25 Mar 2015 14:43:27 -0700 Subject: [PATCH 2/3] Deferring implicit connection behavior from storage.api to Bucket. --- gcloud/storage/__init__.py | 2 +- gcloud/storage/api.py | 32 ++++++++---------------------- gcloud/storage/bucket.py | 29 +++++++++++++++++++++++++++ gcloud/storage/test_api.py | 18 ++++++++++++++--- gcloud/storage/test_bucket.py | 37 +++++++++++++++++++++++++++++++++++ regression/storage.py | 4 ++-- 6 files changed, 92 insertions(+), 30 deletions(-) diff --git a/gcloud/storage/__init__.py b/gcloud/storage/__init__.py index d53861fd7b79..6de23c784512 100644 --- a/gcloud/storage/__init__.py +++ b/gcloud/storage/__init__.py @@ -83,7 +83,7 @@ def set_default_bucket(bucket=None): bucket_name = os.getenv(_BUCKET_ENV_VAR_NAME) connection = get_default_connection() - if bucket_name is not None and connection is not None: + if bucket_name is not None: bucket = Bucket(bucket_name, connection=connection) if bucket is not None: diff --git a/gcloud/storage/api.py b/gcloud/storage/api.py index 4783dcfa9763..44647a1b2917 100644 --- a/gcloud/storage/api.py +++ b/gcloud/storage/api.py @@ -46,14 +46,11 @@ def lookup_bucket(bucket_name, connection=None): :type connection: :class:`gcloud.storage.connection.Connection` or ``NoneType`` :param connection: Optional. The connection to use when sending requests. - If not provided, falls back to default. + If not provided, Bucket() will fall back to default. :rtype: :class:`gcloud.storage.bucket.Bucket` :returns: The bucket matching the name provided or None if not found. """ - if connection is None: - connection = get_default_connection() - try: return get_bucket(bucket_name, connection=connection) except NotFound: @@ -116,15 +113,11 @@ def get_bucket(bucket_name, connection=None): :type connection: :class:`gcloud.storage.connection.Connection` or ``NoneType`` :param connection: Optional. The connection to use when sending requests. - If not provided, falls back to default. + If not provided, Bucket() will fall back to default. :rtype: :class:`gcloud.storage.bucket.Bucket` :returns: The bucket matching the name provided. - :raises: :class:`gcloud.exceptions.NotFound` """ - if connection is None: - connection = get_default_connection() - bucket = Bucket(bucket_name, connection=connection) bucket._reload_properties() return bucket @@ -143,6 +136,9 @@ def create_bucket(bucket_name, project=None, connection=None): This implements "storage.buckets.insert". + If the bucket already exists, will raise + :class:`gcloud.exceptions.Conflict`. + :type project: string :param project: Optional. The project to use when creating bucket. If not provided, falls back to default. @@ -153,25 +149,13 @@ def create_bucket(bucket_name, project=None, connection=None): :type connection: :class:`gcloud.storage.connection.Connection` or ``NoneType`` :param connection: Optional. The connection to use when sending requests. - If not provided, falls back to default. + If not provided, Bucket() will fall back to default. :rtype: :class:`gcloud.storage.bucket.Bucket` :returns: The newly created bucket. - :raises: :class:`gcloud.exceptions.Conflict` if - there is a confict (bucket already exists, invalid name, etc.) """ - if connection is None: - connection = get_default_connection() - if project is None: - project = get_default_project() - - query_params = {'project': project} - response = connection.api_request(method='POST', path='/b', - query_params=query_params, - data={'name': bucket_name}) - name = response.get('name') - bucket = Bucket(name, connection=connection) - bucket._properties = response + bucket = Bucket(bucket_name, connection=connection) + bucket.create(project) return bucket diff --git a/gcloud/storage/bucket.py b/gcloud/storage/bucket.py index 8116c8d8bd0f..d74b797db323 100644 --- a/gcloud/storage/bucket.py +++ b/gcloud/storage/bucket.py @@ -36,6 +36,7 @@ import os import six +from gcloud._helpers import get_default_project from gcloud.exceptions import NotFound from gcloud.storage._helpers import _PropertyMixin from gcloud.storage._helpers import _scalar_property @@ -127,6 +128,34 @@ def exists(self): except NotFound: return False + def create(self, project=None): + """Creates current bucket. + + If the bucket already exists, will raise + :class:`gcloud.exceptions.Conflict`. + + This implements "storage.buckets.insert". + + :type project: string + :param project: Optional. The project to use when creating bucket. + If not provided, falls back to default. + + :rtype: :class:`gcloud.storage.bucket.Bucket` + :returns: The newly created bucket. + :raises: :class:`EnvironmentError` if the project is not given and + can't be inferred. + """ + if project is None: + project = get_default_project() + if project is None: + raise EnvironmentError('Project could not be inferred ' + 'from environment.') + + query_params = {'project': project} + self._properties = self.connection.api_request( + method='POST', path='/b', query_params=query_params, + data={'name': self.name}) + @property def acl(self): """Create our ACL on demand.""" diff --git a/gcloud/storage/test_api.py b/gcloud/storage/test_api.py index 11db48572a67..f3b642015be0 100644 --- a/gcloud/storage/test_api.py +++ b/gcloud/storage/test_api.py @@ -62,11 +62,15 @@ def _lookup_bucket_hit_helper(self, use_default=False): if use_default: with _monkey_defaults(connection=conn): bucket = self._callFUT(BLOB_NAME) + # In the default case, the bucket never has a connection + # bound to it. + self.assertTrue(bucket._connection is None) else: bucket = self._callFUT(BLOB_NAME, connection=conn) + # In the explicit case, the bucket has a connection bound to it. + self.assertTrue(bucket._connection is conn) self.assertTrue(isinstance(bucket, Bucket)) - self.assertTrue(bucket.connection is conn) self.assertEqual(bucket.name, BLOB_NAME) self.assertEqual(http._called_with['method'], 'GET') self.assertEqual(http._called_with['uri'], URI) @@ -187,11 +191,15 @@ def _get_bucket_hit_helper(self, use_default=False): if use_default: with _monkey_defaults(connection=conn): bucket = self._callFUT(BLOB_NAME) + # In the default case, the bucket never has a connection + # bound to it. + self.assertTrue(bucket._connection is None) else: bucket = self._callFUT(BLOB_NAME, connection=conn) + # In the explicit case, the bucket has a connection bound to it. + self.assertTrue(bucket._connection is conn) self.assertTrue(isinstance(bucket, Bucket)) - self.assertTrue(bucket.connection is conn) self.assertEqual(bucket.name, BLOB_NAME) self.assertEqual(http._called_with['method'], 'GET') self.assertEqual(http._called_with['uri'], URI) @@ -232,11 +240,15 @@ def _create_bucket_success_helper(self, project, use_default=False): with _base_monkey_defaults(project=project): with _monkey_defaults(connection=conn): bucket = self._callFUT(BLOB_NAME) + # In the default case, the bucket never has a connection + # bound to it. + self.assertTrue(bucket._connection is None) else: bucket = self._callFUT(BLOB_NAME, project=project, connection=conn) + # In the explicit case, the bucket has a connection bound to it. + self.assertTrue(bucket._connection is conn) self.assertTrue(isinstance(bucket, Bucket)) - self.assertTrue(bucket.connection is conn) self.assertEqual(bucket.name, BLOB_NAME) self.assertEqual(http._called_with['method'], 'POST') self.assertEqual(http._called_with['uri'], URI) diff --git a/gcloud/storage/test_bucket.py b/gcloud/storage/test_bucket.py index bfc84765cab4..85c09037f915 100644 --- a/gcloud/storage/test_bucket.py +++ b/gcloud/storage/test_bucket.py @@ -182,6 +182,43 @@ def api_request(cls, *args, **kwargs): expected_cw = [((), expected_called_kwargs)] self.assertEqual(_FakeConnection._called_with, expected_cw) + def test_create_no_project(self): + from gcloud._testing import _monkey_defaults + BUCKET_NAME = 'bucket-name' + bucket = self._makeOne(BUCKET_NAME) + with _monkey_defaults(project=None): + self.assertRaises(EnvironmentError, bucket.create) + + def test_create_hit_explicit_project(self): + BUCKET_NAME = 'bucket-name' + DATA = {'name': BUCKET_NAME} + connection = _Connection(DATA) + PROJECT = 'PROJECT' + bucket = self._makeOne(BUCKET_NAME, connection=connection) + bucket.create(PROJECT) + + kw, = connection._requested + self.assertEqual(kw['method'], 'POST') + self.assertEqual(kw['path'], '/b') + self.assertEqual(kw['query_params'], {'project': PROJECT}) + self.assertEqual(kw['data'], DATA) + + def test_create_hit_implicit_project(self): + from gcloud._testing import _monkey_defaults + BUCKET_NAME = 'bucket-name' + DATA = {'name': BUCKET_NAME} + connection = _Connection(DATA) + PROJECT = 'PROJECT' + bucket = self._makeOne(BUCKET_NAME, connection=connection) + with _monkey_defaults(project=PROJECT): + bucket.create() + + kw, = connection._requested + self.assertEqual(kw['method'], 'POST') + self.assertEqual(kw['path'], '/b') + self.assertEqual(kw['query_params'], {'project': PROJECT}) + self.assertEqual(kw['data'], DATA) + def test_acl_property(self): from gcloud.storage.acl import BucketACL bucket = self._makeOne() diff --git a/regression/storage.py b/regression/storage.py index 195c5f660b7b..1c5558e69fc0 100644 --- a/regression/storage.py +++ b/regression/storage.py @@ -51,9 +51,9 @@ def setUp(self): self.case_buckets_to_delete = [] def tearDown(self): - with storage.Batch() as batch: + with storage.Batch(): for bucket_name in self.case_buckets_to_delete: - storage.Bucket(bucket_name, connection=batch).delete() + storage.Bucket(bucket_name).delete() def test_create_bucket(self): new_bucket_name = 'a-new-bucket' From adc470b952719237f4b2606a15e58a3fcaa5eb30 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Wed, 25 Mar 2015 14:58:03 -0700 Subject: [PATCH 3/3] Make storage.get_all_buckets() respect implicit/explicit cnxn. --- gcloud/storage/api.py | 12 ++++++++---- gcloud/storage/test_api.py | 13 ++++++++++--- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/gcloud/storage/api.py b/gcloud/storage/api.py index 44647a1b2917..7a16105163dc 100644 --- a/gcloud/storage/api.py +++ b/gcloud/storage/api.py @@ -76,13 +76,12 @@ def get_all_buckets(project=None, connection=None): :type connection: :class:`gcloud.storage.connection.Connection` or ``NoneType`` :param connection: Optional. The connection to use when sending requests. - If not provided, falls back to default. + If not provided, _BucketIterator() will fall back to + default. :rtype: iterable of :class:`gcloud.storage.bucket.Bucket` objects. :returns: All buckets belonging to this project. """ - if connection is None: - connection = get_default_connection() if project is None: project = get_default_project() extra_params = {'project': project} @@ -171,6 +170,11 @@ class _BucketIterator(Iterator): """ def __init__(self, connection, extra_params=None): + # If an implicit connection was intended, we pass along `None` to the + # Bucket() constructor as well. + self._ctor_connection = connection + if connection is None: + connection = get_default_connection() super(_BucketIterator, self).__init__(connection=connection, path='/b', extra_params=extra_params) @@ -182,6 +186,6 @@ def get_items_from_response(self, response): """ for item in response.get('items', []): name = item.get('name') - bucket = Bucket(name, connection=self.connection) + bucket = Bucket(name, connection=self._ctor_connection) bucket._properties = item yield bucket diff --git a/gcloud/storage/test_api.py b/gcloud/storage/test_api.py index f3b642015be0..35b03653e2be 100644 --- a/gcloud/storage/test_api.py +++ b/gcloud/storage/test_api.py @@ -282,19 +282,26 @@ def test_get_items_from_response_empty(self): iterator = self._makeOne(connection) self.assertEqual(list(iterator.get_items_from_response({})), []) - def test_get_items_from_response_non_empty(self): + def _get_items_helper(self, connection): from gcloud.storage.bucket import Bucket BLOB_NAME = 'blob-name' response = {'items': [{'name': BLOB_NAME}]} - connection = object() iterator = self._makeOne(connection) buckets = list(iterator.get_items_from_response(response)) self.assertEqual(len(buckets), 1) bucket = buckets[0] self.assertTrue(isinstance(bucket, Bucket)) - self.assertTrue(bucket.connection is connection) + self.assertTrue(bucket._connection is connection) self.assertEqual(bucket.name, BLOB_NAME) + def test_get_items_from_response_non_empty(self): + connection = object() + self._get_items_helper(connection) + + def test_get_items_from_response_implicit_connection(self): + connection = None + self._get_items_helper(connection) + class Http(object):