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.

Sign up for GitHub

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

Jump to bottom

refactor(storage): deprecate Bucket.create() and requester_pays #9893

Closed
wants to merge 8 commits into from
Closed

refactor(storage): deprecate Bucket.create() and requester_pays #9893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6873209
refactor(storage): deprecate Bucket.create() and requester_pays.
Nov 26, 2019
fe3642d
refactor(storage): deprecate Bucket.create() and requester_pays
Nov 26, 2019
cab1017
Merge branch 'deprecate_create_bucket' of https://github.com/q-logic/…
Nov 26, 2019
e44fad4
Fix requester_pays assertion.
Nov 26, 2019
e26c745
Add checks for deprecation warnings.
Nov 26, 2019
d28d1ec
Add more checks.
Nov 26, 2019
8f401f9
Fix coverage problem.
Nov 26, 2019
53e542e
Lint fix.
Nov 26, 2019
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
3 changes: 1 addition & 2 deletions storage/docs/snippets.py
View file
Open in desktop
Expand Up @@ -51,8 +51,7 @@ def storage_get_started(client, to_delete):
@snippet
def client_bucket_acl(client, to_delete):
bucket_name = "system-test-bucket"
bucket = client.bucket(bucket_name)
bucket.create()
client.create_bucket(bucket_name)

# [START client_bucket_acl]
client = storage.Client()
Expand Down
49 changes: 13 additions & 36 deletions storage/google/cloud/storage/bucket.py
View file
Open in desktop
Expand Up @@ -681,7 +681,7 @@ def create(
predefined_acl=None,
predefined_default_object_acl=None,
):
"""Creates current bucket.
"""DEPRECATED. Creates current bucket.

If the bucket already exists, will raise
:class:`google.cloud.exceptions.Conflict`.
Expand Down Expand Up @@ -718,43 +718,20 @@ 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.")

warnings.warn(
"Bucket.create() is deprecated and will be removed in future."
"Use Client.create_bucket() instead.",
PendingDeprecationWarning,
stacklevel=1,
)
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(
bucket_or_name=self,
project=project,
location=location,
predefined_acl=predefined_acl,
predefined_default_object_acl=predefined_default_object_acl,
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleting old implementation and mirroring to Client.create_bucket()

)
self._set_properties(api_response)

def patch(self, client=None):
"""Sends all changed properties in a PATCH request.
Expand Down
11 changes: 9 additions & 2 deletions storage/google/cloud/storage/client.py
View file
Open in desktop
Expand Up @@ -14,6 +14,8 @@

"""Client for interacting with the Google Cloud Storage API."""

import warnings

import google.api_core.client_options

from google.auth.credentials import AnonymousCredentials
Expand Down Expand Up @@ -348,8 +350,8 @@ def create_bucket(
]):
The bucket resource to pass or name to create.
requester_pays (bool):
Optional. Whether requester pays for API requests for this
bucket and its blobs.
DEPRECATED. 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.
If not passed, uses the project set on the client.
Expand Down Expand Up @@ -405,6 +407,11 @@ def create_bucket(
raise ValueError("Client project not set: pass an explicit project.")

if requester_pays is not None:
warnings.warn(
"requester_pays arg is deprecated.",
PendingDeprecationWarning,
stacklevel=1,
)
bucket.requester_pays = requester_pays

query_params = {"project": project}
Expand Down
240 changes: 29 additions & 211 deletions storage/tests/unit/test_bucket.py
View file
Open in desktop
Expand Up @@ -607,217 +607,6 @@ def api_request(cls, *args, **kwargs):
expected_cw = [((), expected_called_kwargs)]
self.assertEqual(_FakeConnection._called_with, expected_cw)

def test_create_w_user_project(self):
Copy link
Author

@IlyaFaer IlyaFaer Nov 26, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most part of these tests had already been moved into client_test.py in this PR. Some will be moved with the current one

from google.cloud.storage.client import Client

PROJECT = "PROJECT"
BUCKET_NAME = "bucket-name"
USER_PROJECT = "user-project-123"

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"

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

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

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 = _make_connection(
DATA, "{'location': 'us-central1', 'name': 'bucket-name'}"
)

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

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

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 = _make_connection(DATA)
client = Client(project=PROJECT)
client._base_connection = connection

bucket = self._make_one(client=client, name=BUCKET_NAME)
bucket.create()

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 = [
{
"maxAgeSeconds": 60,
"methods": ["*"],
"origin": ["https://example.com/frontend"],
"responseHeader": ["X-Custom-Header"],
}
]
LIFECYCLE_RULES = [{"action": {"type": "Delete"}, "condition": {"age": 365}}]
LOCATION = "eu"
LABELS = {"color": "red", "flavor": "cherry"}
STORAGE_CLASS = "NEARLINE"
DATA = {
"name": BUCKET_NAME,
"cors": CORS,
"lifecycle": {"rule": LIFECYCLE_RULES},
"location": LOCATION,
"storageClass": STORAGE_CLASS,
"versioning": {"enabled": True},
"billing": {"requesterPays": True},
"labels": LABELS,
}

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
bucket.storage_class = STORAGE_CLASS
bucket.versioning_enabled = True
bucket.requester_pays = True
bucket.labels = LABELS
bucket.create(location=LOCATION)

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(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(project=PROJECT)
client._base_connection = connection
bucket = self._make_one(client=client, name=BUCKET_NAME)
bucket.create(predefined_acl="publicRead")

kw, = connection._requested
self.assertEqual(kw["method"], "POST")
self.assertEqual(kw["path"], "/b")
expected_qp = {"project": PROJECT, "predefinedAcl": "publicRead"}
self.assertEqual(kw["query_params"], expected_qp)
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(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(project=PROJECT)
client._base_connection = connection
bucket = self._make_one(client=client, name=BUCKET_NAME)
bucket.create(predefined_default_object_acl="publicRead")

kw, = connection._requested
self.assertEqual(kw["method"], "POST")
self.assertEqual(kw["path"], "/b")
expected_qp = {"project": PROJECT, "predefinedDefaultObjectAcl": "publicRead"}
self.assertEqual(kw["query_params"], expected_qp)
self.assertEqual(kw["data"], DATA)

def test_acl_property(self):
from google.cloud.storage.acl import BucketACL

Expand Down Expand Up @@ -1964,6 +1753,35 @@ def test_versioning_enabled_setter(self):
bucket.versioning_enabled = True
self.assertTrue(bucket.versioning_enabled)

@mock.patch("warnings.warn")
def test_create_deprecated(self, mock_warn):
from google.cloud.storage.client import Client

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

bucket = self._make_one(client=client, name=BUCKET_NAME)
bucket.create()

connection.api_request.assert_called_once_with(
method="POST",
path="/b",
query_params={"project": PROJECT},
data=DATA,
_target_object=bucket,
)

mock_warn.assert_called_with(
"Bucket.create() is deprecated and will be removed in future."
"Use Client.create_bucket() instead.",
PendingDeprecationWarning,
stacklevel=1,
)

def test_requester_pays_getter_missing(self):
NAME = "name"
bucket = self._make_one(name=NAME)
Expand Down