Repository navigation
feat(storagecontrol): add rapid cache samples and integration tests - #16512
kalragauri wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces integration tests and code samples for the Rapid Cache Ultra feature, along with the necessary CI and build configuration updates. The review feedback highlights several violations of the repository's style guide, primarily concerning the over-use of auto where explicit types (such as StatusOr<T> or primitives) are required, as well as a case of swallowing a potential error during bucket listing instead of handling it explicitly.
| auto constexpr kStaleBucketAge = std::chrono::hours(48); | ||
|
|
||
| // Deleting a bucket may fail for a short time after its cache is disabled. | ||
| auto constexpr kDeleteBucketAttempts = 5; |
There was a problem hiding this comment.
According to the repository style guide (Type Deduction (auto)), we should disallow auto for primitives and require explicit numeric and scalar types rather than deducing them from literals. Please use int instead of auto.
int constexpr kDeleteBucketAttempts = 5;References
- Disallow auto for Primitives: Require explicit numeric and scalar types (std::size_t, std::int64_t, bool, etc.) rather than deducing them from literals. (link)
| std::make_unique<StorageControlClient>(MakeStorageControlConnection()); | ||
| RemoveStaleRapidCacheBuckets(); | ||
|
|
||
| auto bucket = CreateRapidCacheBucket(); |
There was a problem hiding this comment.
According to the repository style guide (Type Deduction (auto)), we should reject auto when it hides StatusOr<T>, domain objects, or function return types. Please use the explicit type StatusOr<std::string> instead of auto.
StatusOr<std::string> bucket = CreateRapidCacheBucket();References
- Reject Obscured Domain & Return Types: Flag and reject auto when it hides StatusOr, domain objects, protobuf messages/fields, or function return types. (link)
| auto metadata = MakeBucketIntegrationTestClient().CreateBucketForProject( | ||
| name, project_id_, | ||
| BucketMetadata() | ||
| .set_location(region_name_) | ||
| .set_hierarchical_namespace(BucketHierarchicalNamespace{true}) | ||
| .set_iam_configuration(BucketIamConfiguration{ | ||
| UniformBucketLevelAccess{true, {}}, std::nullopt})); |
There was a problem hiding this comment.
According to the repository style guide (Type Deduction (auto)), we should reject auto when it hides StatusOr<T>, domain objects, or function return types. Please use the explicit type StatusOr<BucketMetadata> instead of auto.
StatusOr<BucketMetadata> metadata =
MakeBucketIntegrationTestClient().CreateBucketForProject(
name, project_id_,
BucketMetadata()
.set_location(region_name_)
.set_hierarchical_namespace(BucketHierarchicalNamespace{true})
.set_iam_configuration(BucketIamConfiguration{
UniformBucketLevelAccess{true, {}}, std::nullopt}));References
- Reject Obscured Domain & Return Types: Flag and reject auto when it hides StatusOr, domain objects, protobuf messages/fields, or function return types. (link)
| return; | ||
| } | ||
| } | ||
| auto client = MakeBucketIntegrationTestClient(); |
There was a problem hiding this comment.
According to the repository style guide (Type Deduction (auto)), we should reject auto when it hides domain objects or function return types. Please use the explicit type Client instead of auto.
Client client = MakeBucketIntegrationTestClient();References
- Reject Obscured Domain & Return Types: Flag and reject auto when it hides StatusOr, domain objects, protobuf messages/fields, or function return types. (link)
| for (auto& bucket : | ||
| MakeBucketIntegrationTestClient().ListBucketsForProject(project_id_)) { |
There was a problem hiding this comment.
According to the repository style guide (Type Deduction (auto)), we should reject auto when it hides StatusOr<T>, domain objects, or function return types. Please use the explicit type StatusOr<BucketMetadata> const& instead of auto& to avoid hiding the type and to prevent unnecessary copies.
for (StatusOr<BucketMetadata> const& bucket :
MakeBucketIntegrationTestClient().ListBucketsForProject(project_id_)) {References
- Reject Obscured Domain & Return Types: Flag and reject auto when it hides StatusOr, domain objects, protobuf messages/fields, or function return types. (link)
| cache.mutable_ttl()->set_seconds(86400); // 24 hours | ||
|
|
||
| auto const parent = "projects/_/buckets/" + bucket_name; | ||
| auto operation = client.CreateRapidCache(parent, cache).get(); |
There was a problem hiding this comment.
According to the repository style guide (Type Deduction (auto)), we should reject auto when it hides StatusOr<T>, domain objects, or function return types. Please use the explicit type google::cloud::StatusOr<google::storage::control::v2::RapidCache> instead of auto.
google::cloud::StatusOr<google::storage::control::v2::RapidCache>
operation = client.CreateRapidCache(parent, cache).get();References
- Reject Obscured Domain & Return Types: Flag and reject auto when it hides StatusOr, domain objects, protobuf messages/fields, or function return types. (link)
| namespace storagecontrol = google::cloud::storagecontrol_v2; | ||
| [](storagecontrol::StorageControlClient client, | ||
| std::string const& cache_name) { | ||
| auto cache = client.GetRapidCache(cache_name); |
There was a problem hiding this comment.
According to the repository style guide (Type Deduction (auto)), we should reject auto when it hides StatusOr<T>, domain objects, or function return types. Please use the explicit type google::cloud::StatusOr<google::storage::control::v2::RapidCache> instead of auto.
google::cloud::StatusOr<google::storage::control::v2::RapidCache> cache =
client.GetRapidCache(cache_name);References
- Reject Obscured Domain & Return Types: Flag and reject auto when it hides StatusOr, domain objects, protobuf messages/fields, or function return types. (link)
| [](storagecontrol::StorageControlClient client, | ||
| std::string const& bucket_name) { | ||
| auto const parent = std::string{"projects/_/buckets/"} + bucket_name; | ||
| for (auto cache : client.ListRapidCaches(parent)) { |
There was a problem hiding this comment.
According to the repository style guide (Type Deduction (auto)), we should reject auto when it hides StatusOr<T>, domain objects, or function return types. Also, using auto without reference qualifiers causes unnecessary copies of non-fundamental types. Please use the explicit type google::cloud::StatusOr<google::storage::control::v2::RapidCache> const& instead of auto.
for (google::cloud::StatusOr<google::storage::control::v2::RapidCache> const&
cache : client.ListRapidCaches(parent)) {References
- Reject Obscured Domain & Return Types: Flag and reject auto when it hides StatusOr, domain objects, protobuf messages/fields, or function return types. (link)
| cache.set_name(cache_name); | ||
| cache.mutable_ttl()->set_seconds(172800); // 48 hours | ||
|
|
||
| auto operation = client.UpdateRapidCache(cache, field_mask).get(); |
There was a problem hiding this comment.
According to the repository style guide (Type Deduction (auto)), we should reject auto when it hides StatusOr<T>, domain objects, or function return types. Please use the explicit type google::cloud::StatusOr<google::storage::control::v2::RapidCache> instead of auto.
google::cloud::StatusOr<google::storage::control::v2::RapidCache>
operation = client.UpdateRapidCache(cache, field_mask).get();References
- Reject Obscured Domain & Return Types: Flag and reject auto when it hides StatusOr, domain objects, protobuf messages/fields, or function return types. (link)
| namespace storagecontrol = google::cloud::storagecontrol_v2; | ||
| [](storagecontrol::StorageControlClient client, | ||
| std::string const& cache_name) { | ||
| auto operation = client.DisableRapidCache(cache_name).get(); |
There was a problem hiding this comment.
According to the repository style guide (Type Deduction (auto)), we should reject auto when it hides StatusOr<T>, domain objects, or function return types. Please use the explicit type google::cloud::StatusOr<google::storage::control::v2::RapidCache> instead of auto.
google::cloud::StatusOr<google::storage::control::v2::RapidCache>
operation = client.DisableRapidCache(cache_name).get();References
- Reject Obscured Domain & Return Types: Flag and reject auto when it hides StatusOr, domain objects, protobuf messages/fields, or function return types. (link)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #16512 +/- ##
==========================================
- Coverage 92.35% 92.34% -0.02%
==========================================
Files 2260 2260
Lines 216842 216842
==========================================
- Hits 200268 200239 -29
- Misses 16574 16603 +29 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
No description provided.