Skip to content

Commit

Permalink
Make 'project' optional / overridable for storage client (googleapis#…
Browse files Browse the repository at this point in the history
…4381)


* Honor explicit 'project=None' for client.

* Add explicit 'project' param to'list_buckets'.

* Add explicit 'project' param to 'Bucket.create' / 'Client.create_buck…

* Enforce that 'topic_project' is passed if 'Client.project' is None.

Closes googleapis#4239
  • Loading branch information
tseaver authored and aw committed Nov 15, 2017
1 parent 5236dd7 commit 9605da8
Show file tree
Hide file tree
Showing 6 changed files with 172 additions and 10 deletions.
20 changes: 18 additions & 2 deletions storage/google/cloud/storage/bucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ def exists(self, client=None):
except NotFound:
return False

def create(self, client=None):
def create(self, client=None, project=None):
"""Creates current bucket.
If the bucket already exists, will raise
Expand All @@ -265,12 +265,28 @@ def create(self, client=None):
``NoneType``
:param client: Optional. The client to use. If not passed, falls back
to the ``client`` stored on the current bucket.
:type project: str
:param project: (Optional) the project under which the bucket is to
be created. If not passed, uses the project set on
the client.
:raises ValueError: if :attr:`user_project` is set.
:raises ValueError: if ``project`` is None and client's
:attr:`project` is also None.
"""
if self.user_project is not None:
raise ValueError("Cannot create bucket with 'user_project' set.")

client = self._require_client(client)
query_params = {'project': client.project}

if project is None:
project = client.project

if project is None:
raise ValueError(
"Client project not set: pass an explicit project.")

query_params = {'project': project}
properties = {key: self._properties[key] for key in self._changes}
properties['name'] = self.name
api_response = client._connection.api_request(
Expand Down
42 changes: 36 additions & 6 deletions storage/google/cloud/storage/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,13 @@
from google.cloud.storage.bucket import Bucket


_marker = object()


class Client(ClientWithProject):
"""Client to bundle configuration needed for API requests.
:type project: str
:type project: str or None
:param project: the project which the client acts on behalf of. Will be
passed when creating a topic. If not passed,
falls back to the default inferred from the environment.
Expand All @@ -55,10 +58,19 @@ class Client(ClientWithProject):
'https://www.googleapis.com/auth/devstorage.read_write')
"""The scopes required for authenticating as a Cloud Storage consumer."""

def __init__(self, project=None, credentials=None, _http=None):
def __init__(self, project=_marker, credentials=None, _http=None):
self._base_connection = None
if project is None:
no_project = True
project = '<none>'
else:
no_project = False
if project is _marker:
project = None
super(Client, self).__init__(project=project, credentials=credentials,
_http=_http)
if no_project:
self.project = None
self._connection = Connection(self)
self._batch_stack = _LocalStack()

Expand Down Expand Up @@ -216,7 +228,7 @@ def lookup_bucket(self, bucket_name):
except NotFound:
return None

def create_bucket(self, bucket_name, requester_pays=None):
def create_bucket(self, bucket_name, requester_pays=None, project=None):
"""Create a new bucket.
For example:
Expand All @@ -238,17 +250,22 @@ def create_bucket(self, bucket_name, requester_pays=None):
(Optional) Whether requester pays for API requests for this
bucket and its blobs.
:type project: str
:param project: (Optional) the project under which the bucket is to
be created. If not passed, uses the project set on
the client.
: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)
bucket.create(client=self, project=project)
return bucket

def list_buckets(self, max_results=None, page_token=None, prefix=None,
projection='noAcl', fields=None):
projection='noAcl', fields=None, project=None):
"""Get all buckets in the project associated to the client.
This will not populate the list of blobs available in each
Expand Down Expand Up @@ -284,11 +301,24 @@ def list_buckets(self, max_results=None, page_token=None, prefix=None,
response with just the next page token and the language of each
bucket returned: 'items/id,nextPageToken'
:type project: str
:param project: (Optional) the project whose buckets are to be listed.
If not passed, uses the project set on the client.
:rtype: :class:`~google.api_core.page_iterator.Iterator`
:raises ValueError: if both ``project`` is ``None`` and the client's
project is also ``None``.
:returns: Iterator of all :class:`~google.cloud.storage.bucket.Bucket`
belonging to this project.
"""
extra_params = {'project': self.project}
if project is None:
project = self.project

if project is None:
raise ValueError(
"Client project not set: pass an explicit project.")

extra_params = {'project': project}

if prefix is not None:
extra_params['prefix'] = prefix
Expand Down
5 changes: 5 additions & 0 deletions storage/google/cloud/storage/notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ def __init__(self, bucket, topic_name,

if topic_project is None:
topic_project = bucket.client.project

if topic_project is None:
raise ValueError(
"Client project not set: pass an explicit topic_project.")

self._topic_project = topic_project

self._properties = {}
Expand Down
27 changes: 27 additions & 0 deletions storage/tests/unit/test_bucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,33 @@ def test_create_w_user_project(self):
with self.assertRaises(ValueError):
bucket.create()

def test_create_w_missing_client_project(self):
BUCKET_NAME = 'bucket-name'
USER_PROJECT = 'user-project-123'
connection = _Connection()
client = _Client(connection, project=None)
bucket = self._make_one(client, BUCKET_NAME)

with self.assertRaises(ValueError):
bucket.create()

def test_create_w_explicit_project(self):
PROJECT = 'PROJECT'
BUCKET_NAME = 'bucket-name'
OTHER_PROJECT = 'other-project-123'
DATA = {'name': BUCKET_NAME}
connection = _Connection(DATA)
client = _Client(connection, project=PROJECT)
bucket = self._make_one(client, BUCKET_NAME)

bucket.create(project=OTHER_PROJECT)

kw, = connection._requested
self.assertEqual(kw['method'], 'POST')
self.assertEqual(kw['path'], '/b')
self.assertEqual(kw['query_params'], {'project': OTHER_PROJECT})
self.assertEqual(kw['data'], DATA)

def test_create_hit(self):
PROJECT = 'PROJECT'
BUCKET_NAME = 'bucket-name'
Expand Down
81 changes: 79 additions & 2 deletions storage/tests/unit/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,38 @@ def test_ctor_connection_type(self):
self.assertIsNone(client.current_batch)
self.assertEqual(list(client._batch_stack), [])

def test_ctor_wo_project(self):
from google.cloud.storage._http import Connection

PROJECT = 'PROJECT'
CREDENTIALS = _make_credentials()

ddp_patch = mock.patch(
'google.cloud.client._determine_default_project',
return_value=PROJECT)

with ddp_patch:
client = self._make_one(credentials=CREDENTIALS)

self.assertEqual(client.project, PROJECT)
self.assertIsInstance(client._connection, Connection)
self.assertIs(client._connection.credentials, CREDENTIALS)
self.assertIsNone(client.current_batch)
self.assertEqual(list(client._batch_stack), [])

def test_ctor_w_project_explicit_none(self):
from google.cloud.storage._http import Connection

CREDENTIALS = _make_credentials()

client = self._make_one(project=None, credentials=CREDENTIALS)

self.assertIsNone(client.project)
self.assertIsInstance(client._connection, Connection)
self.assertIs(client._connection.credentials, CREDENTIALS)
self.assertIsNone(client.current_batch)
self.assertEqual(list(client._batch_stack), [])

def test_create_anonymous_client(self):
from google.auth.credentials import AnonymousCredentials
from google.cloud.storage._http import Connection
Expand Down Expand Up @@ -286,6 +318,7 @@ def test_create_bucket_conflict(self):
from google.cloud.exceptions import Conflict

PROJECT = 'PROJECT'
OTHER_PROJECT = 'OTHER_PROJECT'
CREDENTIALS = _make_credentials()
client = self._make_one(project=PROJECT, credentials=CREDENTIALS)

Expand All @@ -294,15 +327,17 @@ def test_create_bucket_conflict(self):
client._connection.API_BASE_URL,
'storage',
client._connection.API_VERSION,
'b?project=%s' % (PROJECT,),
'b?project=%s' % (OTHER_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, BUCKET_NAME)
with self.assertRaises(Conflict):
client.create_bucket(BUCKET_NAME, project=OTHER_PROJECT)

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']
Expand Down Expand Up @@ -337,6 +372,13 @@ def test_create_bucket_success(self):
json_sent = http.request.call_args_list[0][1]['data']
self.assertEqual(sent, json.loads(json_sent))

def test_list_buckets_wo_project(self):
CREDENTIALS = _make_credentials()
client = self._make_one(project=None, credentials=CREDENTIALS)

with self.assertRaises(ValueError):
client.list_buckets()

def test_list_buckets_empty(self):
from six.moves.urllib.parse import parse_qs
from six.moves.urllib.parse import urlparse
Expand Down Expand Up @@ -371,6 +413,41 @@ def test_list_buckets_empty(self):
uri_parts = urlparse(requested_url)
self.assertEqual(parse_qs(uri_parts.query), expected_query)

def test_list_buckets_explicit_project(self):
from six.moves.urllib.parse import parse_qs
from six.moves.urllib.parse import urlparse

PROJECT = 'PROJECT'
OTHER_PROJECT = 'OTHER_PROJECT'
CREDENTIALS = _make_credentials()
client = self._make_one(project=PROJECT, credentials=CREDENTIALS)

http = _make_requests_session([_make_json_response({})])
client._http_internal = http

buckets = list(client.list_buckets(project=OTHER_PROJECT))

self.assertEqual(len(buckets), 0)

http.request.assert_called_once_with(
method='GET', url=mock.ANY, data=mock.ANY, headers=mock.ANY)

requested_url = http.request.mock_calls[0][2]['url']
expected_base_url = '/'.join([
client._connection.API_BASE_URL,
'storage',
client._connection.API_VERSION,
'b',
])
self.assertTrue(requested_url.startswith(expected_base_url))

expected_query = {
'project': [OTHER_PROJECT],
'projection': ['noAcl'],
}
uri_parts = urlparse(requested_url)
self.assertEqual(parse_qs(uri_parts.query), expected_query)

def test_list_buckets_non_empty(self):
PROJECT = 'PROJECT'
CREDENTIALS = _make_credentials()
Expand Down
7 changes: 7 additions & 0 deletions storage/tests/unit/test_notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,13 @@ def _make_bucket(self, client, name=BUCKET_NAME, user_project=None):
bucket.user_project = user_project
return bucket

def test_ctor_w_missing_project(self):
client = self._make_client(project=None)
bucket = self._make_bucket(client)

with self.assertRaises(ValueError):
self._make_one(bucket, self.TOPIC_NAME)

def test_ctor_defaults(self):
from google.cloud.storage.notification import NONE_PAYLOAD_FORMAT

Expand Down

0 comments on commit 9605da8

Please sign in to comment.