feat(storage): support storage_class via Blob in AsyncAppendableObjectWriter - #18318
feat(storage): support storage_class via Blob in AsyncAppendableObjectWriter#18318chandra-siri wants to merge 14 commits into
Conversation
Add support for specifying storage_class 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. - Add unit tests covering initialization and open stream requests.
![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 specifying the storage_class (such as RAPID for RCU system tests) in asynchronous object writers and streams, along with corresponding unit and system test updates. The review feedback highlights two key improvements: first, ensuring that storage_class is respected in _AsyncWriteObjectStream even when a blob is present, and second, replacing the XOR operator with or in the system test skipif condition to prevent tests from being incorrectly skipped when both Zonal and RCU tests are enabled.
| generation: Optional[int] = None, | ||
| write_handle: Optional[_storage_v2.BidiWriteHandle] = None, | ||
| writer_options: Optional[dict] = None, | ||
| storage_class: Optional[str] = None, |
There was a problem hiding this comment.
instead of adding parameter everytime there's a new configuration, wouldn't it be better to accept something like Blob which would have an option to set these fields?
There was a problem hiding this comment.
yes, we do have that option as well. see method from_blob in this file.
But we also need to support this method as well.
| write_handle: Optional[_storage_v2.BidiWriteHandle] = None, | ||
| routing_token: Optional[str] = None, | ||
| blob: Optional[Blob] = None, | ||
| storage_class: Optional[str] = None, |
There was a problem hiding this comment.
In this case is it possible that we would ignore the storage class specified in blob?
There was a problem hiding this comment.
if blob is present blob's storage class will override the storage_class provided. See line no. 123/128 in this file. Also _AsyncWriteObjectStream is private. Users are not expected to interact with this.
| mock_appendable_writer["mock_client"], | ||
| storage_class=storage_class, | ||
| ) | ||
| assert writer.storage_class == storage_class |
There was a problem hiding this comment.
Could you also assert that when _do_open() (or open()) is called asynchronously, the storage_class property correctly propagates into the _AsyncWriteObjectStream initialization.
…/google-cloud-python into feat/storage-class-via-blob
2 participants
Description
Adds support for propagating
storage_classwhen creating anAsyncAppendableObjectWriterviaAsyncAppendableObjectWriter.from_blob(client, blob), mappingstorage_classin_grpc_conversions.blob_to_proto, and updating system and unit tests accordingly.Changes
"storage_class": "storage_class"to_BLOB_ATTR_TO_PROTO_FIELDin_grpc_conversions.py.storage_class=blob.storage_classinAsyncAppendableObjectWriter.from_blob.storage_classmapping intest__grpc_conversions.py.storage_classhandling inAsyncAppendableObjectWriter.from_blobintest_async_appendable_object_writer.py.test_write_from_blobandtest_write_blob_with_contextsunder RCU by settingblob.storage_class = "RAPID"whenRCU_SYSTEM_TESTSis active.