Repository navigation
chore(storage): rename zonal bucket samples to bidi and rapid bucket - #14698
chandra-siri wants to merge 1 commit into
Conversation
product-auto-label
Bot
added
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 transitions the zonal system tests to rapid and bidirectional streaming (bidi) tests, updating the test scripts, Cloud Build configuration, and sample snippets to use rapid buckets. The feedback recommends defining _RAPID_BUCKET in the Cloud Build substitutions block and passing it dynamically in the SSH command rather than hardcoding it to _ZONAL_BUCKET, ensuring a smoother transition for future triggers.
| # Name of the rapid bucket used by the bidi samples tests. Set by the trigger. | ||
| _ZONAL_BUCKET: "" | ||
| # Service account attached to the test VM. Set by the trigger. | ||
| _ZONAL_VM_SERVICE_ACCOUNT: "" |
There was a problem hiding this comment.
To allow future Cloud Build triggers to transition to using _RAPID_BUCKET directly without breaking, define _RAPID_BUCKET in the substitutions block. This ensures that when the trigger is updated to pass _RAPID_BUCKET, Cloud Build won't fail due to an undefined substitution variable.
# Name of the rapid bucket used by the bidi samples tests. Set by the trigger.
_RAPID_BUCKET: ""
_ZONAL_BUCKET: ""
# Service account attached to the test VM. Set by the trigger.
_ZONAL_VM_SERVICE_ACCOUNT: ""| # Capture the exit code to ensure cleanup happens before the build fails. | ||
| set +e | ||
| gcloud compute ssh ${_VM_NAME} --zone=${_ZONE} --internal-ip --ssh-key-file=/workspace/.ssh/google_compute_engine --command="ulimit -n ${_ULIMIT}; COMMIT_SHA=${COMMIT_SHA} _ZONAL_BUCKET=${_ZONAL_BUCKET} CROSS_REGION_BUCKET=${_CROSS_REGION_BUCKET} _PR_NUMBER=${_PR_NUMBER} bash run_zonal_tests.sh" | ||
| gcloud compute ssh ${_VM_NAME} --zone=${_ZONE} --internal-ip --ssh-key-file=/workspace/.ssh/google_compute_engine --command="ulimit -n ${_ULIMIT}; COMMIT_SHA=${COMMIT_SHA} _RAPID_BUCKET=${_ZONAL_BUCKET} _ZONAL_BUCKET=${_ZONAL_BUCKET} CROSS_REGION_BUCKET=${_CROSS_REGION_BUCKET} _PR_NUMBER=${_PR_NUMBER} bash run_rapid_tests.sh" |
There was a problem hiding this comment.
Pass _RAPID_BUCKET=${_RAPID_BUCKET} instead of hardcoding it to ${_ZONAL_BUCKET}. Since run_rapid_tests.sh already has fallback logic (RAPID_BUCKET="${_RAPID_BUCKET:-${_ZONAL_BUCKET:-}}"), this change allows the script to correctly receive either variable depending on which one is configured in the trigger.
gcloud compute ssh ${_VM_NAME} --zone=${_ZONE} --internal-ip --ssh-key-file=/workspace/.ssh/google_compute_engine --command="ulimit -n ${_ULIMIT}; COMMIT_SHA=${COMMIT_SHA} _RAPID_BUCKET=${_RAPID_BUCKET} _ZONAL_BUCKET=${_ZONAL_BUCKET} CROSS_REGION_BUCKET=${_CROSS_REGION_BUCKET} _PR_NUMBER=${_PR_NUMBER} bash run_rapid_tests.sh"
There was a problem hiding this comment.
Left a few comments across the samples, README, and Cloud Build config:
- Blocking
LintCI fix (storage/samples/snippets/bidi/storage_read_appendable_object_tail.py, lines 17–21):flake8-import-order(I100) fails becausefrom datetime import datetimeandfrom io import BytesIOappear afterimport time(left a file-level comment since this file is a pure rename). Please also runblackon the modified Python files. - Terminology alignment: Use
"a bucket with the Rapid storage class"for bucket requirements (matching Cloud Storage docs andGoogleCloudPlatform/golang-samples#5684/#5815). - README & Cloud Build: Use repo-root
python storage/samples/snippets/bidi/...paths, documentpip install "google-cloud-storage[grpc]", declare_CROSS_REGION_BUCKET: ""alongside_RAPID_BUCKET: "", and fail fast ifRAPID_BUCKETis empty.
(Note: The Kokoro CI check failures are unrelated repo-wide failures from the .kokoro/ removal; the python-sdk-rapid-bucket-samples-test Cloud Build check passed.)
| bucket_name, object_name, grpc_client=None | ||
| ): | ||
| """Uploads an appendable object to zonal bucket. | ||
| """Uploads an appendable object to rapid bucket. |
There was a problem hiding this comment.
Per official Cloud Storage terminology (and Go's storage/bidi/ samples), "Rapid Bucket" is the product name while the bucket requirement is "a bucket with the Rapid storage class". This also fixes the missing article ("to rapid bucket"). Please also run black on this file to remove the extra blank lines around lines 27–28 and 36.
| """Uploads an appendable object to rapid bucket. | |
| """Uploads an appendable object to a bucket with the Rapid storage class. |
| """Uses a pre-warmed pool of writers for appendable objects in a rapid bucket. | ||
|
|
||
| grpc_client: an existing grpc_client to use, this is only for testing. | ||
| """ | ||
| # The ID of your GCS zonal bucket | ||
| # The ID of your GCS rapid bucket | ||
| # bucket_name = "your-unique-bucket-name" |
There was a problem hiding this comment.
Use "a bucket with the Rapid storage class" instead of "a rapid bucket" to match Cloud Storage terminology and Go's optimize_write_latency_pool.go. Note that line 39 is inside the region tag and renders in public docs.
| """Uses a pre-warmed pool of writers for appendable objects in a rapid bucket. | |
| grpc_client: an existing grpc_client to use, this is only for testing. | |
| """ | |
| # The ID of your GCS zonal bucket | |
| # The ID of your GCS rapid bucket | |
| # bucket_name = "your-unique-bucket-name" | |
| """Uses a pre-warmed pool of writers for appendable objects in a bucket with the Rapid storage class. | |
| grpc_client: an existing grpc_client to use, this is only for testing. | |
| """ | |
| # The ID of your GCS bucket (must use the Rapid storage class) | |
| # bucket_name = "your-unique-bucket-name" |
| parser.add_argument( | ||
| "--bucket_name", help="Your Cloud Storage zonal bucket name." | ||
| "--bucket_name", help="Your Cloud Storage rapid bucket name." | ||
| ) |
There was a problem hiding this comment.
Update the --bucket_name help text to use "Rapid storage class" wording. Please also run black on this file so CI does not reformat lines 64–70 and 122–124.
| parser.add_argument( | |
| "--bucket_name", help="Your Cloud Storage zonal bucket name." | |
| "--bucket_name", help="Your Cloud Storage rapid bucket name." | |
| ) | |
| parser.add_argument( | |
| "--bucket_name", | |
| help="Your Cloud Storage bucket name (must use the Rapid storage class).", | |
| ) |
| ## Prerequisites | ||
|
|
||
| - A Google Cloud Platform project with the Cloud Storage API enabled. | ||
| - For the appendable object snippets (create, finalize, pause and resume, tail, | ||
| and the pre-warmed writer pool), a rapid bucket (a bucket with the Rapid | ||
| storage class). | ||
| - The ranged read snippets work with any Google Cloud Storage bucket. | ||
|
|
||
| ## Running the snippets | ||
|
|
||
| ### Create and write to an appendable object | ||
|
|
||
| This snippet uploads an appendable object to a rapid bucket. | ||
|
|
||
| ```bash | ||
| python samples/snippets/bidi/storage_create_and_write_appendable_object.py --bucket_name <bucket_name> --object_name <object_name> | ||
| ``` |
There was a problem hiding this comment.
Three usability fixes for this README:
- Invocation paths:
python samples/snippets/bidi/...only works if the usercds intostorage/first. Please usepython storage/samples/snippets/bidi/...(from repo root, matchinggoogleapis/google-cloud-python#18859) across all 9 commands, or add acd storagestep under Setup. - Missing
[grpc]dependency prerequisite:storage/samples/snippets/requirements.txtinstallsgoogle-cloud-storagewithout thegrpcextra. Please documentpip install "google-cloud-storage[grpc]"under Prerequisites. - Terminology & formatting: Replace
"a rapid bucket"(lines 11, 19, 87) with"a bucket with the Rapid storage class", replace"Google Cloud Platform"with"Google Cloud", remove extra blank lines (lines 48–49, 57–58, 74–76), and add a trailing newline at EOF.
| ## Prerequisites | |
| - A Google Cloud Platform project with the Cloud Storage API enabled. | |
| - For the appendable object snippets (create, finalize, pause and resume, tail, | |
| and the pre-warmed writer pool), a rapid bucket (a bucket with the Rapid | |
| storage class). | |
| - The ranged read snippets work with any Google Cloud Storage bucket. | |
| ## Running the snippets | |
| ### Create and write to an appendable object | |
| This snippet uploads an appendable object to a rapid bucket. | |
| ```bash | |
| python samples/snippets/bidi/storage_create_and_write_appendable_object.py --bucket_name <bucket_name> --object_name <object_name> | |
| ``` | |
| ## Prerequisites | |
| - A Google Cloud project with the Cloud Storage API enabled. | |
| - The Cloud Storage library installed with gRPC support: | |
| ```bash | |
| pip install "google-cloud-storage[grpc]" | |
| ``` | |
| - For the appendable object snippets (create, finalize, pause and resume, tail, | |
| and the pre-warmed writer pool), a bucket with the Rapid storage class. | |
| - The ranged read snippets work with any Google Cloud Storage bucket. | |
| ## Running the snippets | |
| Run the commands below from the repository root: | |
| ### Create and write to an appendable object | |
| This snippet uploads an appendable object to a bucket with the Rapid storage class. | |
| ```bash | |
| python storage/samples/snippets/bidi/storage_create_and_write_appendable_object.py --bucket_name <bucket_name> --object_name <object_name> | |
| ``` |
| """Downloads a range of bytes from multiple objects concurrently. | ||
| Example usage: | ||
| ```python samples/snippets/zonal_buckets/storage_open_multiple_objects_ranged_read.py \ | ||
| ```python samples/snippets/bidi/storage_open_multiple_objects_ranged_read.py \ | ||
| --bucket_name <BUCKET_NAME> \ | ||
| --object_names <OBJECT_1> <OBJECT_2> <OBJECT_3>``` | ||
| """ |
There was a problem hiding this comment.
Update the example path to work from the repository root, remove the markdown backticks (which print literally in --help), and add a blank line after the module docstring to satisfy black.
| """Downloads a range of bytes from multiple objects concurrently. | |
| Example usage: | |
| ```python samples/snippets/zonal_buckets/storage_open_multiple_objects_ranged_read.py \ | |
| ```python samples/snippets/bidi/storage_open_multiple_objects_ranged_read.py \ | |
| --bucket_name <BUCKET_NAME> \ | |
| --object_names <OBJECT_1> <OBJECT_2> <OBJECT_3>``` | |
| """ | |
| """Downloads a range of bytes from multiple objects concurrently. | |
| Example usage: | |
| python storage/samples/snippets/bidi/storage_open_multiple_objects_ranged_read.py \ | |
| --bucket_name <BUCKET_NAME> \ | |
| --object_names <OBJECT_1> <OBJECT_2> <OBJECT_3> | |
| """ | |
| # Name of the rapid bucket used by the bidi samples tests. Set by the trigger. | ||
| _ZONAL_BUCKET: "" | ||
| # Service account attached to the test VM. Set by the trigger. | ||
| _ZONAL_VM_SERVICE_ACCOUNT: "" |
There was a problem hiding this comment.
+1 to adding _RAPID_BUCKET: "" here and passing _RAPID_BUCKET=${_RAPID_BUCKET} on line 125 so Cloud Build's strict substitution check allows updating the trigger later.
Please also declare _CROSS_REGION_BUCKET: "" here (since line 125 references ${_CROSS_REGION_BUCKET} and existing triggers may still pass it), or consider adding _RAPID_VM_SERVICE_ACCOUNT: "" if the trigger's service account substitution will also be renamed.
# Name of the bucket (Rapid storage class) used by the bidi samples tests.
_RAPID_BUCKET: ""
_ZONAL_BUCKET: ""
_CROSS_REGION_BUCKET: ""
# Service account attached to the test VM. Set by the trigger.
_ZONAL_VM_SERVICE_ACCOUNT: ""| export RAPID_BUCKET="${_RAPID_BUCKET:-${_ZONAL_BUCKET:-}}" | ||
| export ZONAL_BUCKET="${RAPID_BUCKET}" |
There was a problem hiding this comment.
Since zb-system-tests-cloudbuild.yaml now defaults _ZONAL_BUCKET to "", RAPID_BUCKET can silently evaluate to an empty string without triggering set -u. Failing fast here avoids running all 9 pytest tests against an empty bucket name if trigger substitutions are misconfigured.
| export RAPID_BUCKET="${_RAPID_BUCKET:-${_ZONAL_BUCKET:-}}" | |
| export ZONAL_BUCKET="${RAPID_BUCKET}" | |
| export RAPID_BUCKET="${_RAPID_BUCKET:-${_ZONAL_BUCKET:-}}" | |
| : "${RAPID_BUCKET:?Either _RAPID_BUCKET or _ZONAL_BUCKET must be set and non-empty}" | |
| export ZONAL_BUCKET="${RAPID_BUCKET}" |
| reason="Rapid bucket (bidi) system tests need to be explicitly enabled. This helps scheduling tests in Kokoro and Cloud Build.", | ||
| ) | ||
|
|
||
|
|
||
| # TODO: replace this with a fixture once zonal bucket creation / deletion | ||
| # TODO: replace this with a fixture once rapid bucket creation / deletion | ||
| # is supported in grpc client or json client client. | ||
| _ZONAL_BUCKET = os.getenv("ZONAL_BUCKET") | ||
| _RAPID_BUCKET = os.getenv("RAPID_BUCKET") or os.getenv("ZONAL_BUCKET") |
There was a problem hiding this comment.
Minor cleanups here:
- Mention
RUN_BIDI_SYSTEM_TESTS=Truein theskipifreason (Kokoro is no longer active on this repo) and fix the"json client client"typo on line 52. - Optionally assert or fail clearly if tests are enabled while
_RAPID_BUCKETis unset, so local runs get an actionable error message.
| reason="Rapid bucket (bidi) system tests need to be explicitly enabled. This helps scheduling tests in Kokoro and Cloud Build.", | |
| ) | |
| # TODO: replace this with a fixture once zonal bucket creation / deletion | |
| # TODO: replace this with a fixture once rapid bucket creation / deletion | |
| # is supported in grpc client or json client client. | |
| _ZONAL_BUCKET = os.getenv("ZONAL_BUCKET") | |
| _RAPID_BUCKET = os.getenv("RAPID_BUCKET") or os.getenv("ZONAL_BUCKET") | |
| reason="Bidi system tests must be explicitly enabled (e.g., RUN_BIDI_SYSTEM_TESTS=True).", | |
| ) | |
| # TODO: replace this with a fixture once creating a bucket with the Rapid | |
| # storage class is supported in the test setup. | |
| _RAPID_BUCKET = os.getenv("RAPID_BUCKET") or os.getenv("ZONAL_BUCKET") |
There was a problem hiding this comment.
[Blocking — Fixes Lint CI] (lines 17–21): flake8-import-order (--import-order-style=google) requires standard library imports to be sorted alphabetically by module name. Both datetime and io must appear before time.
import argparse
import asyncio
from datetime import datetime
from io import BytesIO
import time
Description
Towards b/571318929: Rename GCS Python documentation samples from
zonal_bucketstobidito align with Bidi Standard GA.Changes:
storage/samples/snippets/zonal_buckets/->storage/samples/snippets/bidi/(matching Go's SDK conventionstorage/bidi/).zonal_snippets_test.py->bidi_snippets_test.py.storage/cloudbuild/run_zonal_tests.sh->storage/cloudbuild/run_rapid_tests.sh.bidi/README.md: Drop "Zonal Buckets" title/prose; clarify prerequisites (appendable snippets require a rapid bucket / bucket with the Rapid storage class, ranged-read snippets work on any bucket); update all snippet invocation paths tosamples/snippets/bidi/.storage_create_and_write_appendable_object.py: Docstring "to zonal bucket" -> "to rapid bucket".storage_optimize_write_latency_pool.py: Docstrings, comments, and--bucket_nameargparse help updated to reference rapid bucket.storage_open_multiple_objects_ranged_read.py: Module docstring example path updated tosamples/snippets/bidi/.# [START storage_...]and# [END storage_...]region tags remain 100% byte-for-byte unchanged.bidi_snippets_test.py: Defines_RAPID_BUCKET = os.getenv("RAPID_BUCKET") or os.getenv("ZONAL_BUCKET"); acceptsRUN_RAPID_SYSTEM_TESTS,RUN_BIDI_SYSTEM_TESTS, orRUN_ZONAL_SYSTEM_TESTS; updates test object prefix tobidi-snippets-test-.zb-system-tests-cloudbuild.yaml: scp/sshrun_rapid_tests.shand pass_RAPID_BUCKET=${_ZONAL_BUCKET}while retaining existing trigger substitutions (_ZONAL_BUCKET,_ZONAL_VM_SERVICE_ACCOUNT) so existing Cloud Build triggers continue functioning seamlessly without modifications.