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

refactor(storage): move 'create_bucket' implementation from 'Bucket' to 'Client' #8604

Merged
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
feccf18
Storage: client redesign proposal
Jul 3, 2019
f9e6924
Merge branch 'client_redesign_create_bucket_7762' into redesign_clien…
Jul 3, 2019
1f57249
Delete unittest.main
Jul 3, 2019
b9146a4
Delete unnecessary import
Jul 3, 2019
be8166c
Cosmetic changes
Jul 3, 2019
c9ec3e9
Merge branch 'master' into redesign_client_create_bucket_7762
Jul 3, 2019
d1bafa1
Merge branch 'master' into redesign_client_create_bucket_7762
Jul 5, 2019
8c21c5a
Merge branch 'redesign_client_create_bucket_7762' of https://github.c…
Jul 5, 2019
8d31e55
Merge branch 'master' into redesign_client_create_bucket_7762
Jul 10, 2019
1c50d4c
Merge branch 'master' into redesign_client_create_bucket_7762
Aug 8, 2019
09d0c6f
Delete public property for bucket properties and mocks in bucket test…
Aug 8, 2019
b8e091f
Merge branch 'master' into redesign_client_create_bucket_7762
Oct 28, 2019
5a31265
Updated PRed changes to actualize them with changes from a79d98de3e9d…
Oct 28, 2019
e4f94da
Black reformat
Oct 28, 2019
e96d05c
Add user_project param into Client.create_bucket() method.
Nov 1, 2019
35427fb
Fix comment.
Nov 1, 2019
608d7ac
Merge branch 'master' of https://github.com/q-logic/google-cloud-pyth…
Nov 4, 2019
db2c0ae
Merge branch 'redesign_client_create_bucket_7762' of https://github.c…
Nov 4, 2019
8000ce6
Create bucket should support user_project.
Nov 4, 2019
e206be2
Test for user_project query parameter.
Nov 4, 2019
2cd2321
Merge branch 'master' into redesign_client_create_bucket_7762
Nov 5, 2019
675e11d
Copy tests from bucket test to client.
Nov 5, 2019
2530133
Revert "Copy tests from bucket test to client."
Nov 5, 2019
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
42 changes: 7 additions & 35 deletions storage/google/cloud/storage/bucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -640,43 +640,15 @@ def create(
Optional. Name of predefined ACL to apply to bucket's objects. See:
https://cloud.google.com/storage/docs/access-control/lists#predefined-acl
"""
if self.user_project is not None:
raise ValueError("Cannot create bucket with 'user_project' set.")

client = self._require_client(client)

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}

if predefined_acl is not None:
predefined_acl = BucketACL.validate_predefined(predefined_acl)
query_params["predefinedAcl"] = predefined_acl

if predefined_default_object_acl is not None:
predefined_default_object_acl = DefaultObjectACL.validate_predefined(
predefined_default_object_acl
)
query_params["predefinedDefaultObjectAcl"] = predefined_default_object_acl

properties = {key: self._properties[key] for key in self._changes}
properties["name"] = self.name

if location is not None:
properties["location"] = location

api_response = client._connection.api_request(
method="POST",
path="/b",
query_params=query_params,
data=properties,
_target_object=self,
client.create_bucket(
self,
project=project,
location=location,
predefined_acl=predefined_acl,
predefined_default_object_acl=predefined_default_object_acl,
user_project=self.user_project,
)
self._set_properties(api_response)

def patch(self, client=None):
"""Sends all changed properties in a PATCH request.
Expand Down
66 changes: 63 additions & 3 deletions storage/google/cloud/storage/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
from google.cloud.storage.bucket import Bucket
from google.cloud.storage.blob import Blob
from google.cloud.storage.hmac_key import HMACKeyMetadata
from google.cloud.storage.acl import BucketACL
from google.cloud.storage.acl import DefaultObjectACL


_marker = object()
Expand Down Expand Up @@ -324,7 +326,16 @@ def lookup_bucket(self, bucket_name):
except NotFound:
return None

def create_bucket(self, bucket_or_name, requester_pays=None, project=None):
def create_bucket(
self,
bucket_or_name,
requester_pays=None,
project=None,
user_project=None,
location=None,
predefined_acl=None,
predefined_default_object_acl=None,
IlyaFaer marked this conversation as resolved.
Show resolved Hide resolved
):
"""API call: create a new bucket via a POST request.

See
Expand All @@ -340,8 +351,21 @@ def create_bucket(self, bucket_or_name, requester_pays=None, project=None):
Optional. Whether requester pays for API requests for this
bucket and its blobs.
project (str):
Optional. the project under which the bucket is to be created.
Optional. The project under which the bucket is to be created.
If not passed, uses the project set on the client.
user_project (str):
Optional. The project ID to be billed for API requests
made via created bucket.
location (str):
Optional. The location of the bucket. If not passed,
the default location, US, will be used. See
https://cloud.google.com/storage/docs/bucket-locations
predefined_acl (str):
Optional. Name of predefined ACL to apply to bucket. See:
https://cloud.google.com/storage/docs/access-control/lists#predefined-acl
predefined_default_object_acl (str):
Optional. Name of predefined ACL to apply to bucket's objects. See:
https://cloud.google.com/storage/docs/access-control/lists#predefined-acl
IlyaFaer marked this conversation as resolved.
Show resolved Hide resolved

Returns:
google.cloud.storage.bucket.Bucket
Expand Down Expand Up @@ -372,11 +396,47 @@ def create_bucket(self, bucket_or_name, requester_pays=None, project=None):
>>> bucket = client.create_bucket(bucket) # API request.

"""
if user_project is not None:
raise ValueError("Cannot create bucket with 'user_project' set.")
IlyaFaer marked this conversation as resolved.
Show resolved Hide resolved

bucket = self._bucket_arg_to_bucket(bucket_or_name)

if project is None:
project = self.project

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

if requester_pays is not None:
bucket.requester_pays = requester_pays
bucket.create(client=self, project=project)

query_params = {"project": project}

if predefined_acl is not None:
predefined_acl = BucketACL.validate_predefined(predefined_acl)
query_params["predefinedAcl"] = predefined_acl

if predefined_default_object_acl is not None:
predefined_default_object_acl = DefaultObjectACL.validate_predefined(
predefined_default_object_acl
)
query_params["predefinedDefaultObjectAcl"] = predefined_default_object_acl

properties = {key: bucket._properties[key] for key in bucket._changes}
properties["name"] = bucket.name

if location is not None:
properties["location"] = location

api_response = self._connection.api_request(
method="POST",
path="/b",
query_params=query_params,
data=properties,
_target_object=bucket,
)

bucket._set_properties(api_response)
return bucket

def download_blob_to_file(self, blob_or_uri, file_obj, start=None, end=None):
Expand Down
129 changes: 91 additions & 38 deletions storage/tests/unit/test_bucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@
import pytest


def _make_connection(*responses):
import google.cloud.storage._http

mock_connection = mock.create_autospec(google.cloud.storage._http.Connection)
mock_connection.user_agent = "testing 1.2.3"
mock_connection.api_request.side_effect = list(responses)
return mock_connection

IlyaFaer marked this conversation as resolved.
Show resolved Hide resolved

def _create_signing_credentials():
import google.auth.credentials

Expand Down Expand Up @@ -534,77 +543,104 @@ def api_request(cls, *args, **kwargs):
self.assertEqual(_FakeConnection._called_with, expected_cw)

def test_create_w_user_project(self):
from google.cloud.storage.client import Client

PROJECT = "PROJECT"
BUCKET_NAME = "bucket-name"
USER_PROJECT = "user-project-123"
connection = _Connection()
client = _Client(connection, project=PROJECT)

client = Client(project=PROJECT)
client._base_connection = _Connection()

bucket = self._make_one(client, BUCKET_NAME, user_project=USER_PROJECT)

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

def test_create_w_missing_client_project(self):
from google.cloud.storage.client import Client

BUCKET_NAME = "bucket-name"
connection = _Connection()
client = _Client(connection, project=None)

client = Client(project=None)
bucket = self._make_one(client, BUCKET_NAME)

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

def test_create_w_explicit_project(self):
from google.cloud.storage.client import Client

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)
connection = _make_connection(DATA)

bucket.create(project=OTHER_PROJECT)
client = Client(project=PROJECT)
client._base_connection = connection

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)
bucket = self._make_one(client, BUCKET_NAME)
bucket.create(project=OTHER_PROJECT)
connection.api_request.assert_called_once_with(
method="POST",
path="/b",
query_params={"project": OTHER_PROJECT},
data=DATA,
_target_object=bucket,
)

def test_create_w_explicit_location(self):
from google.cloud.storage.client import Client

PROJECT = "PROJECT"
BUCKET_NAME = "bucket-name"
LOCATION = "us-central1"
DATA = {"location": LOCATION, "name": BUCKET_NAME}
connection = _Connection(

connection = _make_connection(
DATA, "{'location': 'us-central1', 'name': 'bucket-name'}"
)
client = _Client(connection, project=PROJECT)
bucket = self._make_one(client, BUCKET_NAME)

client = Client(project=PROJECT)
client._base_connection = connection

bucket = self._make_one(client, BUCKET_NAME)
bucket.create(location=LOCATION)

kw, = connection._requested
self.assertEqual(kw["method"], "POST")
self.assertEqual(kw["path"], "/b")
self.assertEqual(kw["data"], DATA)
connection.api_request.assert_called_once_with(
method="POST",
path="/b",
data=DATA,
_target_object=bucket,
query_params={"project": "PROJECT"},
)
self.assertEqual(bucket.location, LOCATION)

def test_create_hit(self):
from google.cloud.storage.client import Client

PROJECT = "PROJECT"
BUCKET_NAME = "bucket-name"
DATA = {"name": BUCKET_NAME}
connection = _Connection(DATA)
client = _Client(connection, project=PROJECT)
connection = _make_connection(DATA)
client = Client(project=PROJECT)
client._base_connection = connection

bucket = self._make_one(client=client, name=BUCKET_NAME)
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)
connection.api_request.assert_called_once_with(
method="POST",
path="/b",
query_params={"project": PROJECT},
data=DATA,
_target_object=bucket,
)

def test_create_w_extra_properties(self):
from google.cloud.storage.client import Client

BUCKET_NAME = "bucket-name"
PROJECT = "PROJECT"
CORS = [
Expand All @@ -629,8 +665,11 @@ def test_create_w_extra_properties(self):
"billing": {"requesterPays": True},
"labels": LABELS,
}
connection = _Connection(DATA)
client = _Client(connection, project=PROJECT)

connection = _make_connection(DATA)
client = Client(project=PROJECT)
client._base_connection = connection

bucket = self._make_one(client=client, name=BUCKET_NAME)
bucket.cors = CORS
bucket.lifecycle_rules = LIFECYCLE_RULES
Expand All @@ -640,29 +679,37 @@ def test_create_w_extra_properties(self):
bucket.labels = LABELS
bucket.create(location=LOCATION)

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)
connection.api_request.assert_called_once_with(
method="POST",
path="/b",
query_params={"project": PROJECT},
data=DATA,
_target_object=bucket,
)

def test_create_w_predefined_acl_invalid(self):
from google.cloud.storage.client import Client

PROJECT = "PROJECT"
BUCKET_NAME = "bucket-name"
DATA = {"name": BUCKET_NAME}
connection = _Connection(DATA)
client = _Client(connection, project=PROJECT)
client = Client(project=PROJECT)
client._base_connection = connection
bucket = self._make_one(client=client, name=BUCKET_NAME)

with self.assertRaises(ValueError):
bucket.create(predefined_acl="bogus")

def test_create_w_predefined_acl_valid(self):
from google.cloud.storage.client import Client

PROJECT = "PROJECT"
BUCKET_NAME = "bucket-name"
DATA = {"name": BUCKET_NAME}
connection = _Connection(DATA)
client = _Client(connection, project=PROJECT)
client = Client(project=PROJECT)
client._base_connection = connection
bucket = self._make_one(client=client, name=BUCKET_NAME)
bucket.create(predefined_acl="publicRead")

Expand All @@ -674,22 +721,28 @@ def test_create_w_predefined_acl_valid(self):
self.assertEqual(kw["data"], DATA)

def test_create_w_predefined_default_object_acl_invalid(self):
from google.cloud.storage.client import Client

PROJECT = "PROJECT"
BUCKET_NAME = "bucket-name"
DATA = {"name": BUCKET_NAME}
connection = _Connection(DATA)
client = _Client(connection, project=PROJECT)
client = Client(project=PROJECT)
client._base_connection = connection
bucket = self._make_one(client=client, name=BUCKET_NAME)

with self.assertRaises(ValueError):
bucket.create(predefined_default_object_acl="bogus")

def test_create_w_predefined_default_object_acl_valid(self):
from google.cloud.storage.client import Client

PROJECT = "PROJECT"
BUCKET_NAME = "bucket-name"
DATA = {"name": BUCKET_NAME}
connection = _Connection(DATA)
client = _Client(connection, project=PROJECT)
client = Client(project=PROJECT)
client._base_connection = connection
bucket = self._make_one(client=client, name=BUCKET_NAME)
bucket.create(predefined_default_object_acl="publicRead")

Expand Down