Repository navigation
feat(storage): add enterprise storage classes to Bucket and Blob STORAGE_CLASSES tuples - #18951
chandra-siri wants to merge 2 commits into
Conversation
…AGE_CLASSES tuples
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for new enterprise storage classes (STANDARD_ENTERPRISE, STANDARD_FLEXBACKUP_ENTERPRISE, and LR_STANDARD_ENTERPRISE) to the Blob and Bucket classes, along with corresponding unit and system tests. The review feedback recommends defining these new storage classes as constants in constants.py to avoid magic strings and maintain consistency. Additionally, it is suggested to refactor the new standalone unit test in test_blob.py into the existing unittest.TestCase structure to match the file's established testing pattern.
| 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, | ||
| ) |
There was a problem hiding this comment.
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.
| 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, | |
| ) |
| 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 |
There was a problem hiding this comment.
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.
| 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 | |
| ) |
| @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} |
There was a problem hiding this comment.
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.
chandra-siri
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
Summary
Adds the Enterprise storage classes (
STANDARD_ENTERPRISE,STANDARD_FLEXBACKUP_ENTERPRISE, andLR_STANDARD_ENTERPRISE) to theBucket.STORAGE_CLASSESandBlob.STORAGE_CLASSESdocumentation tuples ingoogle-cloud-storage:google/cloud/storage/bucket.py: Includes"STANDARD_ENTERPRISE","STANDARD_FLEXBACKUP_ENTERPRISE", and"LR_STANDARD_ENTERPRISE"inBucket.STORAGE_CLASSES.google/cloud/storage/blob.py: Includes"STANDARD_ENTERPRISE","STANDARD_FLEXBACKUP_ENTERPRISE", and"LR_STANDARD_ENTERPRISE"inBlob.STORAGE_CLASSES.tests/unit/test_bucket.py,tests/unit/test_blob.py) and system tests (tests/system/test_bucket.py,tests/system/test_blob.py) using@pytest.mark.parametrizeto verify bucket creation and blob storage class updates with the Enterprise storage classes.Split from #18949 (Part 1 of 2:
Bucket.STORAGE_CLASSESandBlob.STORAGE_CLASSEStuple entries).