Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions packages/google-cloud-storage/google/cloud/storage/blob.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,12 @@ class Blob(_PropertyMixin):

STORAGE_CLASSES = (
STANDARD_STORAGE_CLASS,
"STANDARD_ENTERPRISE",
"STANDARD_FLEXBACKUP_ENTERPRISE",
NEARLINE_STORAGE_CLASS,
COLDLINE_STORAGE_CLASS,
ARCHIVE_STORAGE_CLASS,
"LR_STANDARD_ENTERPRISE",
MULTI_REGIONAL_LEGACY_STORAGE_CLASS,
REGIONAL_LEGACY_STORAGE_CLASS,
)
Comment on lines 192 to 202

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

To maintain consistency and avoid magic strings, the new enterprise storage classes should be defined as constants in google/cloud/storage/constants.py and imported, similar to STANDARD_STORAGE_CLASS and other existing storage classes.

Suggested change
STORAGE_CLASSES = (
STANDARD_STORAGE_CLASS,
"STANDARD_ENTERPRISE",
"STANDARD_FLEXBACKUP_ENTERPRISE",
NEARLINE_STORAGE_CLASS,
COLDLINE_STORAGE_CLASS,
ARCHIVE_STORAGE_CLASS,
"LR_STANDARD_ENTERPRISE",
MULTI_REGIONAL_LEGACY_STORAGE_CLASS,
REGIONAL_LEGACY_STORAGE_CLASS,
)
STORAGE_CLASSES = (
STANDARD_STORAGE_CLASS,
STANDARD_ENTERPRISE_STORAGE_CLASS,
STANDARD_FLEXBACKUP_ENTERPRISE_STORAGE_CLASS,
NEARLINE_STORAGE_CLASS,
COLDLINE_STORAGE_CLASS,
ARCHIVE_STORAGE_CLASS,
LR_STANDARD_ENTERPRISE_STORAGE_CLASS,
MULTI_REGIONAL_LEGACY_STORAGE_CLASS,
REGIONAL_LEGACY_STORAGE_CLASS,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -650,9 +650,12 @@ class Bucket(_PropertyMixin):

STORAGE_CLASSES = (
STANDARD_STORAGE_CLASS,
"STANDARD_ENTERPRISE",
"STANDARD_FLEXBACKUP_ENTERPRISE",
NEARLINE_STORAGE_CLASS,
COLDLINE_STORAGE_CLASS,
ARCHIVE_STORAGE_CLASS,
"LR_STANDARD_ENTERPRISE",
MULTI_REGIONAL_LEGACY_STORAGE_CLASS, # legacy
REGIONAL_LEGACY_STORAGE_CLASS, # legacy
DURABLE_REDUCED_AVAILABILITY_LEGACY_STORAGE_CLASS, # legacy
Comment on lines 651 to 661

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

To maintain consistency and avoid magic strings, the new enterprise storage classes should be defined as constants in google/cloud/storage/constants.py and imported, similar to STANDARD_STORAGE_CLASS and other existing storage classes.

Suggested change
STORAGE_CLASSES = (
STANDARD_STORAGE_CLASS,
"STANDARD_ENTERPRISE",
"STANDARD_FLEXBACKUP_ENTERPRISE",
NEARLINE_STORAGE_CLASS,
COLDLINE_STORAGE_CLASS,
ARCHIVE_STORAGE_CLASS,
"LR_STANDARD_ENTERPRISE",
MULTI_REGIONAL_LEGACY_STORAGE_CLASS, # legacy
REGIONAL_LEGACY_STORAGE_CLASS, # legacy
DURABLE_REDUCED_AVAILABILITY_LEGACY_STORAGE_CLASS, # legacy
STORAGE_CLASSES = (
STANDARD_STORAGE_CLASS,
STANDARD_ENTERPRISE_STORAGE_CLASS,
STANDARD_FLEXBACKUP_ENTERPRISE_STORAGE_CLASS,
NEARLINE_STORAGE_CLASS,
COLDLINE_STORAGE_CLASS,
ARCHIVE_STORAGE_CLASS,
LR_STANDARD_ENTERPRISE_STORAGE_CLASS,
MULTI_REGIONAL_LEGACY_STORAGE_CLASS, # legacy
REGIONAL_LEGACY_STORAGE_CLASS, # legacy
DURABLE_REDUCED_AVAILABILITY_LEGACY_STORAGE_CLASS, # legacy
)

Expand Down
27 changes: 27 additions & 0 deletions packages/google-cloud-storage/tests/system/test_blob.py
Original file line number Diff line number Diff line change
Expand Up @@ -1212,6 +1212,33 @@ def test_blob_update_storage_class_large_file(
assert blob.storage_class == constants.COLDLINE_STORAGE_CLASS


def test_blob_update_storage_class_enterprise(
storage_client, buckets_to_delete, blobs_to_delete, file_data
):
bucket_name = _helpers.unique_name("blob-ent-sc")
bucket = storage_client.bucket(bucket_name)
bucket.storage_class = "STANDARD_ENTERPRISE"

_helpers.retry_429_503(bucket.create)(location="us-central1")
buckets_to_delete.append(bucket)

blob = bucket.blob("EnterpriseSmallFile")
assert "STANDARD_ENTERPRISE" in blob.STORAGE_CLASSES
assert "LR_STANDARD_ENTERPRISE" in blob.STORAGE_CLASSES
info = file_data["simple"]
blob.upload_from_filename(info["path"])
blobs_to_delete.append(blob)
assert blob.storage_class == "STANDARD_ENTERPRISE"

blob.update_storage_class("LR_STANDARD_ENTERPRISE")
blob.reload()
assert blob.storage_class == "LR_STANDARD_ENTERPRISE"

blob.update_storage_class("STANDARD_ENTERPRISE")
blob.reload()
assert blob.storage_class == "STANDARD_ENTERPRISE"


def test_object_retention_lock(storage_client, buckets_to_delete, blobs_to_delete):
from google.cloud.storage._helpers import _NOW, _UTC

Expand Down
27 changes: 27 additions & 0 deletions packages/google-cloud-storage/tests/system/test_bucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,33 @@ def test_bucket_create_w_alt_storage_class(storage_client, buckets_to_delete):
assert created.storage_class == constants.ARCHIVE_STORAGE_CLASS


@pytest.mark.parametrize(
"storage_class,suffix,location",
[
("STANDARD_ENTERPRISE", "std-ent", "us-central1"),
("LR_STANDARD_ENTERPRISE", "lr-std-ent", "us-central1"),
("STANDARD_FLEXBACKUP_ENTERPRISE", "fb-ent", "us"),
],
)
def test_bucket_create_w_enterprise_storage_class(
storage_client, buckets_to_delete, storage_class, suffix, location
):
bucket_name = _helpers.unique_name(f"bucket-w-{suffix}")

with pytest.raises(exceptions.NotFound):
storage_client.get_bucket(bucket_name)

bucket = storage_client.bucket(bucket_name)
assert storage_class in bucket.STORAGE_CLASSES
bucket.storage_class = storage_class

_helpers.retry_429_503(bucket.create)(location=location)
buckets_to_delete.append(bucket)

created = storage_client.get_bucket(bucket_name)
assert created.storage_class == storage_class


def test_bucket_lifecycle_rules(storage_client, buckets_to_delete):
from google.cloud.storage import constants
from google.cloud.storage.bucket import (
Expand Down
19 changes: 19 additions & 0 deletions packages/google-cloud-storage/tests/unit/test_blob.py
Original file line number Diff line number Diff line change
Expand Up @@ -6788,6 +6788,25 @@ def test_w_existing_qs(self):
self.assertEqual(self._call_fut(BASE_URL, NV_LIST), f"{BASE_URL}&{expected}")


@pytest.mark.parametrize(
"storage_class",
[
"STANDARD_ENTERPRISE",
"STANDARD_FLEXBACKUP_ENTERPRISE",
"LR_STANDARD_ENTERPRISE",
],
)
def test_blob_storage_class_enterprise_tuples(storage_class):
from google.cloud.storage.blob import Blob

bucket = _Bucket()
blob = Blob("blob-name", bucket=bucket)
assert storage_class in blob.STORAGE_CLASSES
blob.storage_class = storage_class
assert blob.storage_class == storage_class
assert blob._properties == {"storageClass": storage_class}
Comment on lines +6791 to +6807

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The rest of test_blob.py uses unittest.TestCase classes for structuring unit tests. Defining test_blob_storage_class_enterprise_tuples as a standalone pytest function is inconsistent with the existing test structure. Consider refactoring this test as methods within the appropriate Test_Blob (or similar) test class to maintain consistency.



class _Connection(object):
API_BASE_URL = "http://example.com"
USER_AGENT = "testing 1.2.3"
Expand Down
24 changes: 24 additions & 0 deletions packages/google-cloud-storage/tests/unit/test_bucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -3279,6 +3279,30 @@ def test_storage_class_setter_STANDARD(self):
self.assertEqual(bucket.storage_class, STANDARD_STORAGE_CLASS)
self.assertTrue("storageClass" in bucket._changes)

def test_storage_class_setter_STANDARD_ENTERPRISE(self):
NAME = "name"
bucket = self._make_one(name=NAME)
self.assertIn("STANDARD_ENTERPRISE", bucket.STORAGE_CLASSES)
bucket.storage_class = "STANDARD_ENTERPRISE"
self.assertEqual(bucket.storage_class, "STANDARD_ENTERPRISE")
self.assertTrue("storageClass" in bucket._changes)

def test_storage_class_setter_STANDARD_FLEXBACKUP_ENTERPRISE(self):
NAME = "name"
bucket = self._make_one(name=NAME)
self.assertIn("STANDARD_FLEXBACKUP_ENTERPRISE", bucket.STORAGE_CLASSES)
bucket.storage_class = "STANDARD_FLEXBACKUP_ENTERPRISE"
self.assertEqual(bucket.storage_class, "STANDARD_FLEXBACKUP_ENTERPRISE")
self.assertTrue("storageClass" in bucket._changes)

def test_storage_class_setter_LR_STANDARD_ENTERPRISE(self):
NAME = "name"
bucket = self._make_one(name=NAME)
self.assertIn("LR_STANDARD_ENTERPRISE", bucket.STORAGE_CLASSES)
bucket.storage_class = "LR_STANDARD_ENTERPRISE"
self.assertEqual(bucket.storage_class, "LR_STANDARD_ENTERPRISE")
self.assertTrue("storageClass" in bucket._changes)

def test_storage_class_setter_NEARLINE(self):
from google.cloud.storage.constants import NEARLINE_STORAGE_CLASS

Expand Down
Loading