Skip to content

feat(storage): support storage_class in AsyncAppendableObjectWriter - #18289

Open
chandra-siri wants to merge 1 commit into
googleapis:mainfrom
chandra-siri:feat/storage-class-rapid
Open

feat(storage): support storage_class in AsyncAppendableObjectWriter#18289
chandra-siri wants to merge 1 commit into
googleapis:mainfrom
chandra-siri:feat/storage-class-rapid

Conversation

@chandra-siri

Copy link
Copy Markdown
Contributor

Description

Adds support for specifying storage_class ('STANDARD' or 'RAPID') when writing appendable objects via AsyncAppendableObjectWriter.

Changes

  • Added storage_class parameter (defaulting to None) in AsyncAppendableObjectWriter.__init__ and propagated it to _AsyncWriteObjectStream during open().
  • Added storage_class parameter in _AsyncWriteObjectStream.__init__ and passed it when constructing the _storage_v2.Object resource in open().
  • Added input validation in both classes ensuring storage_class must be either 'STANDARD' or 'RAPID'.
  • Added unit tests in test_async_appendable_object_writer.py and test_async_write_object_stream.py covering initialization, validation, and stream opening for both 'STANDARD' and 'RAPID'.

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.
@chandra-siri
chandra-siri requested a review from a team as a code owner September 4, 2026 14:37

@gemini-code-assist gemini-code-assist Bot left a comment

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.

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")

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 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
  1. Remove duplicate lines of code, especially duplicate assertions in tests, to keep the codebase clean and avoid redundancy.

Comment on lines +191 to +197
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}'"
)

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 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}'"
                )

Comment on lines +86 to +92
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}'"
)

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 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}'"
                )

Comment on lines +143 to +149
@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

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

Update the unit test to verify that lowercase storage class inputs (e.g., 'standard', 'rapid') are correctly accepted and normalized to uppercase.

Suggested change
@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

Comment on lines +68 to +73
@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

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

Update the unit test to verify that lowercase storage class inputs are correctly accepted and normalized to uppercase.

Suggested change
@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant