feat(storage): support storage_class in AsyncAppendableObjectWriter - #18289
feat(storage): support storage_class in AsyncAppendableObjectWriter#18289chandra-siri wants to merge 1 commit into
Conversation
Add support for specifying storage_class ('STANDARD' or 'RAPID') when writing
objects via AsyncAppendableObjectWriter.
- Add storage_class parameter to AsyncAppendableObjectWriter.__init__ and
propagate it to _AsyncWriteObjectStream on open().
- Add storage_class parameter to _AsyncWriteObjectStream.__init__ and pass it
to _storage_v2.Object creation.
- Validate storage_class to ensure only supported values ('STANDARD' or 'RAPID')
are allowed.
- Add unit tests covering initialization, validation, and open stream requests.
There was a problem hiding this comment.
Code Review
This pull request introduces support for specifying a storage_class (specifically 'STANDARD' or 'RAPID') when asynchronously writing or appending to GCS objects. The feedback suggests importing the _SUPPORTED_STORAGE_CLASSES constant to avoid duplication, normalizing the storage_class input to uppercase for robustness, and updating the unit tests to verify this case-insensitive behavior.
| _BIDI_WRITE_REDIRECTED_TYPE_URL = ( | ||
| "type.googleapis.com/google.storage.v2.BidiWriteObjectRedirectedError" | ||
| ) | ||
| _SUPPORTED_STORAGE_CLASSES = ("STANDARD", "RAPID") |
There was a problem hiding this comment.
The constant _SUPPORTED_STORAGE_CLASSES is duplicated here and in async_write_object_stream.py. To improve maintainability and avoid potential out-of-sync issues in the future, consider importing it from google.cloud.storage.asyncio.async_write_object_stream instead of redefining it.
References
- Remove duplicate lines of code, especially duplicate assertions in tests, to keep the codebase clean and avoid redundancy.
| if ( | ||
| storage_class is not None | ||
| and storage_class not in _SUPPORTED_STORAGE_CLASSES | ||
| ): | ||
| raise ValueError( | ||
| f"storage_class must be either 'STANDARD' or 'RAPID', got '{storage_class}'" | ||
| ) |
There was a problem hiding this comment.
To make the API more robust and user-friendly, consider normalizing the storage_class input to uppercase (e.g., converting 'standard' to 'STANDARD'). This prevents unexpected validation failures due to casing.
if storage_class is not None:
storage_class = storage_class.upper()
if storage_class not in _SUPPORTED_STORAGE_CLASSES:
raise ValueError(
f"storage_class must be either 'STANDARD' or 'RAPID', got '{storage_class}'"
)| if ( | ||
| storage_class is not None | ||
| and storage_class not in _SUPPORTED_STORAGE_CLASSES | ||
| ): | ||
| raise ValueError( | ||
| f"storage_class must be either 'STANDARD' or 'RAPID', got '{storage_class}'" | ||
| ) |
There was a problem hiding this comment.
To ensure consistency with AsyncAppendableObjectWriter and handle direct usages of _AsyncWriteObjectStream robustly, normalize the storage_class input to uppercase before validation.
if storage_class is not None:
storage_class = storage_class.upper()
if storage_class not in _SUPPORTED_STORAGE_CLASSES:
raise ValueError(
f"storage_class must be either 'STANDARD' or 'RAPID', got '{storage_class}'"
)| @pytest.mark.parametrize("storage_class", ["STANDARD", "RAPID"]) | ||
| def test_init_with_storage_class(self, mock_appendable_writer, storage_class): | ||
| writer = self._make_one( | ||
| mock_appendable_writer["mock_client"], | ||
| storage_class=storage_class, | ||
| ) | ||
| assert writer.storage_class == storage_class |
There was a problem hiding this comment.
Update the unit test to verify that lowercase storage class inputs (e.g., 'standard', 'rapid') are correctly accepted and normalized to uppercase.
| @pytest.mark.parametrize("storage_class", ["STANDARD", "RAPID"]) | |
| def test_init_with_storage_class(self, mock_appendable_writer, storage_class): | |
| writer = self._make_one( | |
| mock_appendable_writer["mock_client"], | |
| storage_class=storage_class, | |
| ) | |
| assert writer.storage_class == storage_class | |
| @pytest.mark.parametrize( | |
| "storage_class, expected", | |
| [ | |
| ("STANDARD", "STANDARD"), | |
| ("standard", "STANDARD"), | |
| ("RAPID", "RAPID"), | |
| ("rapid", "RAPID"), | |
| ], | |
| ) | |
| def test_init_with_storage_class( | |
| self, mock_appendable_writer, storage_class, expected | |
| ): | |
| writer = self._make_one( | |
| mock_appendable_writer["mock_client"], | |
| storage_class=storage_class, | |
| ) | |
| assert writer.storage_class == expected |
| @pytest.mark.parametrize("storage_class", ["STANDARD", "RAPID"]) | ||
| def test_init_with_storage_class(self, mock_client, storage_class): | ||
| stream = _AsyncWriteObjectStream( | ||
| mock_client, BUCKET, OBJECT, storage_class=storage_class | ||
| ) | ||
| assert stream.storage_class == storage_class |
There was a problem hiding this comment.
Update the unit test to verify that lowercase storage class inputs are correctly accepted and normalized to uppercase.
| @pytest.mark.parametrize("storage_class", ["STANDARD", "RAPID"]) | |
| def test_init_with_storage_class(self, mock_client, storage_class): | |
| stream = _AsyncWriteObjectStream( | |
| mock_client, BUCKET, OBJECT, storage_class=storage_class | |
| ) | |
| assert stream.storage_class == storage_class | |
| @pytest.mark.parametrize( | |
| "storage_class, expected", | |
| [ | |
| ("STANDARD", "STANDARD"), | |
| ("standard", "STANDARD"), | |
| ("RAPID", "RAPID"), | |
| ("rapid", "RAPID"), | |
| ], | |
| ) | |
| def test_init_with_storage_class(self, mock_client, storage_class, expected): | |
| stream = _AsyncWriteObjectStream( | |
| mock_client, BUCKET, OBJECT, storage_class=storage_class | |
| ) | |
| assert stream.storage_class == expected |
Description
Adds support for specifying
storage_class('STANDARD' or 'RAPID') when writing appendable objects viaAsyncAppendableObjectWriter.Changes
storage_classparameter (defaulting toNone) inAsyncAppendableObjectWriter.__init__and propagated it to_AsyncWriteObjectStreamduringopen().storage_classparameter in_AsyncWriteObjectStream.__init__and passed it when constructing the_storage_v2.Objectresource inopen().storage_classmust be either'STANDARD'or'RAPID'.test_async_appendable_object_writer.pyandtest_async_write_object_stream.pycovering initialization, validation, and stream opening for both'STANDARD'and'RAPID'.