feat(storage): add builder and writer skeleton for open appendable object#5699
feat(storage): add builder and writer skeleton for open appendable object#5699vsharonlynn wants to merge 1 commit into
Conversation
product-auto-label
Bot
added
the
api: storage
![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 a skeleton implementation for appendable object uploads, including the OpenAppendableObject request builder and the AppendableObjectWriter utility. Feedback focuses on the structural requirement for the writer to hold a reference to the storage transport layer (the stub) to facilitate bidirectional streaming. Additionally, the reviewer noted that the use of #[allow(dead_code)] without a corresponding TODO or rationale violates the repository's style guide regarding temporary lint suppressions.
| Ok(AppendableObjectWriter { | ||
| bucket: resource.bucket, | ||
| object: resource.name, | ||
| params: self.request.params, | ||
| if_metageneration_match: self.request.spec.if_metageneration_match, | ||
| if_metageneration_not_match: self.request.spec.if_metageneration_not_match, | ||
| options: self.options, | ||
| }) |
There was a problem hiding this comment.
The AppendableObjectWriter is intended to handle bidirectional streaming RPCs, but it is currently instantiated without the storage stub (self.stub). To perform I/O operations, the writer will need access to the transport layer. Consider passing the stub to the writer and making the writer generic over the storage implementation S to maintain consistency with other writers in the SDK (like WriteObject).
| #[cfg(google_cloud_unstable_storage_bidi)] | ||
| #[derive(Debug)] | ||
| pub struct AppendableObjectWriter { | ||
| pub(crate) bucket: String, | ||
| pub(crate) object: String, | ||
| pub(crate) params: Option<crate::model::CommonObjectRequestParams>, | ||
| pub(crate) if_metageneration_match: Option<i64>, | ||
| pub(crate) if_metageneration_not_match: Option<i64>, | ||
| pub(crate) options: RequestOptions, | ||
| } |
There was a problem hiding this comment.
The AppendableObjectWriter struct lacks a field for the storage stub and is not generic over the storage implementation S. Since this writer is responsible for the bidirectional streaming RPC, it needs a reference to the transport layer (the stub) to function correctly.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5699 +/- ##
==========================================
- Coverage 97.91% 97.75% -0.16%
==========================================
Files 226 228 +2
Lines 55347 55647 +300
==========================================
+ Hits 54191 54398 +207
- Misses 1156 1249 +93 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
This follows PR #5697.