feat(storage): add support for bucket metadata call in async client#16262
feat(storage): add support for bucket metadata call in async client#16262bajajneha27 wants to merge 8 commits 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 implements the asynchronous GetBucket API across the Google Cloud Storage C++ SDK, including client interfaces, connection implementations, decorators, stubs, mocks, and tests. A review of the changes identified an issue in the integration tests where BucketName is constructed with an already-prefixed string, which would result in a double-prefixed bucket name.
|
|
||
| auto bucket = | ||
| async | ||
| .GetBucket(BucketName("projects/_/buckets/" + MakeRandomObjectName())) |
There was a problem hiding this comment.
The BucketName constructor expects the simple bucket name (e.g., my-bucket), and its FullName() method automatically prepends the projects/_/buckets/ prefix. Passing a string that already contains the prefix will result in a double-prefixed name like projects/_/buckets/projects/_/buckets/.... You should pass only the random name to BucketName.
.GetBucket(BucketName(MakeRandomObjectName()))
There was a problem hiding this comment.
This seems like a valid finding. Please fix.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #16262 +/- ##
==========================================
- Coverage 92.27% 92.27% -0.01%
==========================================
Files 2214 2214
Lines 206537 206710 +173
==========================================
+ Hits 190589 190745 +156
- Misses 15948 15965 +17 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
d6a54d9 to
aff7403
Compare
|
|
||
| auto bucket = | ||
| async | ||
| .GetBucket(BucketName("projects/_/buckets/" + MakeRandomObjectName())) |
There was a problem hiding this comment.
This seems like a valid finding. Please fix.
| std::shared_ptr<grpc::ClientContext> context, | ||
| google::cloud::internal::ImmutableOptions options, | ||
| google::storage::v2::GetBucketRequest const& request) { | ||
| AddIdempotencyToken(*context, id); |
There was a problem hiding this comment.
These tokens are intended for mutating operations (like WriteObject, DeleteObject) to prevent duplicate execution on the server side. Read operations are naturally idempotent and do not need to generate or send this token.
0f51cbd to
23ec33e
Compare
There was a problem hiding this comment.
missed this the last time: pls add a unit test for changes in this file.
2 participants
No description provided.