Skip to content

Commit

Permalink
Add 'Bucket.requester_pays' property. (#3488)
Browse files Browse the repository at this point in the history
Also, add 'requester_pays' argument to 'Client.create_bucket'.

Add a system test which exercises the feature.

Note that the new system test is skipped, because 'Buckets.insert' fails
with the 'billing/requesterPays' field set, both in our system tests and
in the 'Try It!' form in the docs.

Toward #3474.
  • Loading branch information
tseaver committed Sep 22, 2017
1 parent 5a0fe35 commit 3394eca
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 19 deletions.
32 changes: 31 additions & 1 deletion storage/google/cloud/storage/bucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -897,10 +897,40 @@ def versioning_enabled(self, value):
details.
:type value: convertible to boolean
:param value: should versioning be anabled for the bucket?
:param value: should versioning be enabled for the bucket?
"""
self._patch_property('versioning', {'enabled': bool(value)})

@property
def requester_pays(self):
"""Does the requester pay for API requests for this bucket?
.. note::
No public docs exist yet for the "requester pays" feature.
:setter: Update whether requester pays for this bucket.
:getter: Query whether requester pays for this bucket.
:rtype: bool
:returns: True if requester pays for API requests for the bucket,
else False.
"""
versioning = self._properties.get('billing', {})
return versioning.get('requesterPays', False)

@requester_pays.setter
def requester_pays(self, value):
"""Update whether requester pays for API requests for this bucket.
See https://cloud.google.com/storage/docs/<DOCS-MISSING> for
details.
:type value: convertible to boolean
:param value: should requester pay for API requests for the bucket?
"""
self._patch_property('billing', {'requesterPays': bool(value)})

def configure_website(self, main_page_suffix=None, not_found_page=None):
"""Configure website-related properties.
Expand Down
9 changes: 8 additions & 1 deletion storage/google/cloud/storage/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ def lookup_bucket(self, bucket_name):
except NotFound:
return None

def create_bucket(self, bucket_name):
def create_bucket(self, bucket_name, requester_pays=None):
"""Create a new bucket.
For example:
Expand All @@ -211,10 +211,17 @@ def create_bucket(self, bucket_name):
:type bucket_name: str
:param bucket_name: The bucket name to create.
:type requester_pays: bool
:param requester_pays:
(Optional) Whether requester pays for API requests for this
bucket and its blobs.
:rtype: :class:`google.cloud.storage.bucket.Bucket`
:returns: The newly created bucket.
"""
bucket = Bucket(self, name=bucket_name)
if requester_pays is not None:
bucket.requester_pays = requester_pays
bucket.create(client=self)
return bucket

Expand Down
12 changes: 12 additions & 0 deletions storage/tests/system.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
from test_utils.system import unique_resource_id


REQUESTER_PAYS_ENABLED = False # query from environment?


def _bad_copy(bad_request):
"""Predicate: pass only exceptions for a failed copyTo."""
err_msg = bad_request.message
Expand Down Expand Up @@ -96,6 +99,15 @@ def test_create_bucket(self):
self.case_buckets_to_delete.append(new_bucket_name)
self.assertEqual(created.name, new_bucket_name)

@unittest.skipUnless(REQUESTER_PAYS_ENABLED, "requesterPays not enabled")
def test_create_bucket_with_requester_pays(self):
new_bucket_name = 'w-requester-pays' + unique_resource_id('-')
created = Config.CLIENT.create_bucket(
new_bucket_name, requester_pays=True)
self.case_buckets_to_delete.append(new_bucket_name)
self.assertEqual(created.name, new_bucket_name)
self.assertTrue(created.requester_pays)

def test_list_buckets(self):
buckets_to_create = [
'new' + unique_resource_id(),
Expand Down
20 changes: 20 additions & 0 deletions storage/tests/unit/test_bucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ def test_create_w_extra_properties(self):
'location': LOCATION,
'storageClass': STORAGE_CLASS,
'versioning': {'enabled': True},
'billing': {'requesterPays': True},
'labels': LABELS,
}
connection = _Connection(DATA)
Expand All @@ -186,6 +187,7 @@ def test_create_w_extra_properties(self):
bucket.location = LOCATION
bucket.storage_class = STORAGE_CLASS
bucket.versioning_enabled = True
bucket.requester_pays = True
bucket.labels = LABELS
bucket.create()

Expand Down Expand Up @@ -916,6 +918,24 @@ def test_versioning_enabled_setter(self):
bucket.versioning_enabled = True
self.assertTrue(bucket.versioning_enabled)

def test_requester_pays_getter_missing(self):
NAME = 'name'
bucket = self._make_one(name=NAME)
self.assertEqual(bucket.requester_pays, False)

def test_requester_pays_getter(self):
NAME = 'name'
before = {'billing': {'requesterPays': True}}
bucket = self._make_one(name=NAME, properties=before)
self.assertEqual(bucket.requester_pays, True)

def test_requester_pays_setter(self):
NAME = 'name'
bucket = self._make_one(name=NAME)
self.assertFalse(bucket.requester_pays)
bucket.requester_pays = True
self.assertTrue(bucket.requester_pays)

def test_configure_website_defaults(self):
NAME = 'name'
UNSET = {'website': {'mainPageSuffix': None,
Expand Down
41 changes: 24 additions & 17 deletions storage/tests/unit/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,23 +184,23 @@ def test_get_bucket_hit(self):
CREDENTIALS = _make_credentials()
client = self._make_one(project=PROJECT, credentials=CREDENTIALS)

BLOB_NAME = 'blob-name'
BUCKET_NAME = 'bucket-name'
URI = '/'.join([
client._connection.API_BASE_URL,
'storage',
client._connection.API_VERSION,
'b',
'%s?projection=noAcl' % (BLOB_NAME,),
'%s?projection=noAcl' % (BUCKET_NAME,),
])

data = {'name': BLOB_NAME}
data = {'name': BUCKET_NAME}
http = _make_requests_session([_make_json_response(data)])
client._http_internal = http

bucket = client.get_bucket(BLOB_NAME)
bucket = client.get_bucket(BUCKET_NAME)

self.assertIsInstance(bucket, Bucket)
self.assertEqual(bucket.name, BLOB_NAME)
self.assertEqual(bucket.name, BUCKET_NAME)
http.request.assert_called_once_with(
method='GET', url=URI, data=mock.ANY, headers=mock.ANY)

Expand Down Expand Up @@ -234,22 +234,22 @@ def test_lookup_bucket_hit(self):
CREDENTIALS = _make_credentials()
client = self._make_one(project=PROJECT, credentials=CREDENTIALS)

BLOB_NAME = 'blob-name'
BUCKET_NAME = 'bucket-name'
URI = '/'.join([
client._connection.API_BASE_URL,
'storage',
client._connection.API_VERSION,
'b',
'%s?projection=noAcl' % (BLOB_NAME,),
'%s?projection=noAcl' % (BUCKET_NAME,),
])
data = {'name': BLOB_NAME}
data = {'name': BUCKET_NAME}
http = _make_requests_session([_make_json_response(data)])
client._http_internal = http

bucket = client.lookup_bucket(BLOB_NAME)
bucket = client.lookup_bucket(BUCKET_NAME)

self.assertIsInstance(bucket, Bucket)
self.assertEqual(bucket.name, BLOB_NAME)
self.assertEqual(bucket.name, BUCKET_NAME)
http.request.assert_called_once_with(
method='GET', url=URI, data=mock.ANY, headers=mock.ANY)

Expand All @@ -260,21 +260,24 @@ def test_create_bucket_conflict(self):
CREDENTIALS = _make_credentials()
client = self._make_one(project=PROJECT, credentials=CREDENTIALS)

BLOB_NAME = 'blob-name'
BUCKET_NAME = 'bucket-name'
URI = '/'.join([
client._connection.API_BASE_URL,
'storage',
client._connection.API_VERSION,
'b?project=%s' % (PROJECT,),
])
data = {'error': {'message': 'Conflict'}}
sent = {'name': BUCKET_NAME}
http = _make_requests_session([
_make_json_response(data, status=http_client.CONFLICT)])
client._http_internal = http

self.assertRaises(Conflict, client.create_bucket, BLOB_NAME)
self.assertRaises(Conflict, client.create_bucket, BUCKET_NAME)
http.request.assert_called_once_with(
method='POST', url=URI, data=mock.ANY, headers=mock.ANY)
json_sent = http.request.call_args_list[0][1]['data']
self.assertEqual(sent, json.loads(json_sent))

def test_create_bucket_success(self):
from google.cloud.storage.bucket import Bucket
Expand All @@ -283,23 +286,27 @@ def test_create_bucket_success(self):
CREDENTIALS = _make_credentials()
client = self._make_one(project=PROJECT, credentials=CREDENTIALS)

BLOB_NAME = 'blob-name'
BUCKET_NAME = 'bucket-name'
URI = '/'.join([
client._connection.API_BASE_URL,
'storage',
client._connection.API_VERSION,
'b?project=%s' % (PROJECT,),
])
data = {'name': BLOB_NAME}
sent = {'name': BUCKET_NAME, 'billing': {'requesterPays': True}}
data = sent
http = _make_requests_session([_make_json_response(data)])
client._http_internal = http

bucket = client.create_bucket(BLOB_NAME)
bucket = client.create_bucket(BUCKET_NAME, requester_pays=True)

self.assertIsInstance(bucket, Bucket)
self.assertEqual(bucket.name, BLOB_NAME)
self.assertEqual(bucket.name, BUCKET_NAME)
self.assertTrue(bucket.requester_pays)
http.request.assert_called_once_with(
method='POST', url=URI, data=mock.ANY, headers=mock.ANY)
json_sent = http.request.call_args_list[0][1]['data']
self.assertEqual(sent, json.loads(json_sent))

def test_list_buckets_empty(self):
from six.moves.urllib.parse import parse_qs
Expand Down Expand Up @@ -422,7 +429,7 @@ def test_page_non_empty_response(self):
credentials = _make_credentials()
client = self._make_one(project=project, credentials=credentials)

blob_name = 'blob-name'
blob_name = 'bucket-name'
response = {'items': [{'name': blob_name}]}

def dummy_response():
Expand Down

0 comments on commit 3394eca

Please sign in to comment.