feat(storagecontrol): Added samples for Anywhere Cache (v2)#1
feat(storagecontrol): Added samples for Anywhere Cache (v2)#1nidhiii-27 wants to merge 3 commits into
Conversation
Implemented seven new code samples for the Storage Control API's 'Anywhere Cache' feature: - create_anywhere_cache.py - get_anywhere_cache.py - list_anywhere_caches.py - update_anywhere_cache.py - pause_anywhere_cache.py - resume_anywhere_cache.py - disable_anywhere_cache.py Updates include: - Use of client's path helper methods (`anywhere_cache_path`) for resource names. - Addition of integration tests in snippets_test.py without skip annotations. - Standard LRO blocking with specific comments for real-world applications. - 2026 Copyright headers and appropriate region tags. Co-authored-by: nidhiii-27 <224584462+nidhiii-27@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Code Review
This pull request introduces a suite of Python scripts for managing Google Cloud Storage Anywhere Caches, covering operations such as creation, retrieval, listing, updating, pausing, resuming, and disabling. It also includes corresponding integration tests. The review feedback recommends using the client library's built-in bucket_path helper method for resource name construction instead of hardcoded strings for better consistency. Furthermore, it suggests adding cleanup logic to the test suite to ensure resources are properly disabled even if a test fails.
| storage_control_client = storage_control_v2.StorageControlClient() | ||
| # The storage bucket path uses the global access pattern, in which the "_" | ||
| # denotes this bucket exists in the global namespace. | ||
| bucket_path = f"projects/_/buckets/{bucket_name}" |
There was a problem hiding this comment.
The PR description mentions utilizing built-in path helper methods for resource name construction. While anywhere_cache_path is used in other files, this line uses a hardcoded f-string for the bucket path. For consistency and to follow the client's best practices, consider using the bucket_path helper method.
| bucket_path = f"projects/_/buckets/{bucket_name}" | |
| bucket_path = storage_control_client.bucket_path("_", bucket_name) |
There was a problem hiding this comment.
I've updated all samples to use the client's path helper methods where available. For bucket paths (which don't have a direct helper like anywhere_cache_path), I'm now using managed_folder_path and splitting it to consistently use the client's internal path logic instead of manual f-strings.
There was a problem hiding this comment.
@jules no need to use managed_folder_path ,use this: storage_control_client.bucket_path("_", bucket_name)
There was a problem hiding this comment.
I've checked the StorageControlClient (v1.11.0) and it doesn't seem to have a bucket_path method (it currently only has anywhere_cache_path, folder_path, managed_folder_path, etc.). I've updated the samples to use the standard pattern found in this repository: f"{storage_control_client.common_project_path('_')}/buckets/{bucket_name}". This avoids the managed_folder_path workaround while remaining consistent with other samples like create_folder.py. If bucket_path should be available, please let me know if I need to update to a specific version!
| storage_control_client = storage_control_v2.StorageControlClient() | ||
| # The storage bucket path uses the global access pattern, in which the "_" | ||
| # denotes this bucket exists in the global namespace. | ||
| bucket_path = f"projects/_/buckets/{bucket_name}" |
There was a problem hiding this comment.
Similar to other samples in this PR, consider using the client's built-in bucket_path helper method for resource name construction instead of a hardcoded f-string, aligning with the goal stated in the PR description.
| bucket_path = f"projects/_/buckets/{bucket_name}" | |
| bucket_path = storage_control_client.bucket_path("_", bucket_name) |
| def test_anywhere_cache_operations( | ||
| capsys: pytest.LogCaptureFixture, | ||
| ubla_enabled_bucket: storage.Bucket, | ||
| ) -> None: | ||
| bucket_name = ubla_enabled_bucket.name | ||
| zone = "us-central1-a" | ||
|
|
||
| # Test create anywhere cache | ||
| create_anywhere_cache.create_anywhere_cache( | ||
| bucket_name=bucket_name, zone=zone | ||
| ) | ||
| out, _ = capsys.readouterr() | ||
| assert f"buckets/{bucket_name}/anywhereCaches/{zone}" in out | ||
|
|
||
| # Test get anywhere cache | ||
| get_anywhere_cache.get_anywhere_cache(bucket_name=bucket_name, zone=zone) | ||
| out, _ = capsys.readouterr() | ||
| assert f"buckets/{bucket_name}/anywhereCaches/{zone}" in out | ||
|
|
||
| # Test list anywhere caches | ||
| list_anywhere_caches.list_anywhere_caches(bucket_name=bucket_name) | ||
| out, _ = capsys.readouterr() | ||
| assert f"buckets/{bucket_name}/anywhereCaches/{zone}" in out | ||
|
|
||
| # Test update anywhere cache | ||
| update_anywhere_cache.update_anywhere_cache( | ||
| bucket_name=bucket_name, | ||
| zone=zone, | ||
| admission_policy="admit-on-second-miss", | ||
| ) | ||
| out, _ = capsys.readouterr() | ||
| assert f"buckets/{bucket_name}/anywhereCaches/{zone}" in out | ||
|
|
||
| # Test pause anywhere cache | ||
| pause_anywhere_cache.pause_anywhere_cache( | ||
| bucket_name=bucket_name, zone=zone | ||
| ) | ||
| out, _ = capsys.readouterr() | ||
| assert f"buckets/{bucket_name}/anywhereCaches/{zone}" in out | ||
|
|
||
| # Test resume anywhere cache | ||
| resume_anywhere_cache.resume_anywhere_cache( | ||
| bucket_name=bucket_name, zone=zone | ||
| ) | ||
| out, _ = capsys.readouterr() | ||
| assert f"buckets/{bucket_name}/anywhereCaches/{zone}" in out | ||
|
|
||
| # Test disable anywhere cache | ||
| disable_anywhere_cache.disable_anywhere_cache( | ||
| bucket_name=bucket_name, zone=zone | ||
| ) | ||
| out, _ = capsys.readouterr() | ||
| assert f"buckets/{bucket_name}/anywhereCaches/{zone}" in out | ||
|
|
There was a problem hiding this comment.
The test test_anywhere_cache_operations lacks cleanup logic in case of failure. If an assertion fails before disable_anywhere_cache, the resource remains active, which may cause orphaned resources or prevent the bucket fixture from cleaning up correctly. Consider using a try...finally block to ensure the cache is disabled regardless of the test outcome.
There was a problem hiding this comment.
I've added the try...finally block to the integration test to ensure that disable_anywhere_cache is always called, which helps with resource cleanup even if earlier parts of the test fail or time out.
- Use client path helper methods for resource names in all samples. - Add try...finally block in integration tests for robust resource cleanup. - Ensure consistent code formatting and linting compliance. Co-authored-by: nidhiii-27 <224584462+nidhiii-27@users.noreply.github.com>
- Use client path helper methods for resource names in all samples. - Add try...finally block in integration tests for robust resource cleanup. - Ensure consistent code formatting and linting compliance. - Use standard project/bucket path pattern for parents. Co-authored-by: nidhiii-27 <224584462+nidhiii-27@users.noreply.github.com>
Finalized implementation of seven Python code samples for the Storage Control API's 'Anywhere Cache' feature. This version addresses review feedback by utilizing the client's built-in path helper methods for resource name construction and removing the skip annotations from integration tests. The samples cover the entire resource lifecycle (create, get, list, update, pause, resume, disable) and handle long-running operations synchronously as required for documentation. Comprehensive integration tests are included in
snippets_test.py. Junk files and unrelated changes were removed to ensure a clean patch.PR created automatically by Jules for task 17401725828640608185 started by @nidhiii-27