Repository navigation
Conversation
|
|
jmaeagle99
left a comment
jmaeagle99
left a comment
There was a problem hiding this comment.
Thanks for the contribution! Have a handful of comments. Also, please use the GCS drivers in Go and TypeScript as a reference for GCS behaviors instead of the S3 drivers.
| > ⚠️ **This package is experimental.** ⚠️ | ||
| `GCSStorageDriver` stores and retrieves Temporal payloads in Google Cloud Storage, | ||
| following the Python SDK's [S3 driver](../../aws/s3driver/) conventions. |
There was a problem hiding this comment.
This driver should not be using S3 conventions as S3 != GCS. There should be no references to S3 in this driver whether that's in code or documentation.
| return StorageDriverClaim( | ||
| claim_data={ | ||
| "bucket": bucket, | ||
| "key": key, |
There was a problem hiding this comment.
In the existing GCS drivers (Go, TypeScript), this is object_name.
|
|
||
| async def _download(claim: StorageDriverClaim) -> Payload: | ||
| bucket = claim.claim_data["bucket"] | ||
| key = claim.claim_data["key"] |
There was a problem hiding this comment.
| _KEY_SAFE_CHARS = frozenset(string.ascii_letters + string.digits + "!-_.*'()") | ||
|
|
||
|
|
||
| def _percent_encode(val: str | None) -> str | None: |
There was a problem hiding this comment.
This is the S3 safe set, which is not the same a GCS set. See https://github.com/temporalio/sdk-go/blob/main/contrib/gcp/gcsdriver/driver.go#L292 for the specific algorithm for GCS.
| digest_segments = f"/d/sha256/{hash_digest}" | ||
|
|
||
| key = f"v0{namespace_segment}{context_segments}{digest_segments}" | ||
| if len(key.encode("utf-8")) > 1024: |
There was a problem hiding this comment.
This should not throw because that will continuously fail the workflow, activity, nexus task for something that cannot be fixed by a customer (other than no using the driver). There should be a fallback mechanism in place. See https://github.com/temporalio/sdk-go/blob/main/contrib/gcp/gcsdriver/driver.go#L259 as an example.
| ###### Built-in Drivers | ||
|
|
||
| - **[S3 Storage Driver](temporalio/contrib/aws/s3driver/)**: ⚠️ **Experimental** ⚠️ Amazon S3 driver. Ships with an aioboto3 client, or bring your own by subclassing `S3StorageDriverClient`. | ||
| - **[GCS Storage Driver](temporalio/contrib/gcp/gcsdriver/)**: ⚠️ **Experimental** ⚠️ Google Cloud Storage driver. Ships with a gcloud-aio-storage client adapter, or bring your own by subclassing `GCSStorageDriverClient`. |
There was a problem hiding this comment.
This should be listed first (alphabetical order).
|
|
||
| @pytest.fixture | ||
| async def gcs_storage(gcs_bucket: str) -> AsyncIterator[Storage]: | ||
| endpoint = os.environ.get("STORAGE_EMULATOR_HOST") |
There was a problem hiding this comment.
I would prefer to either use a mocked server or an emulator by default, similar to the S3 testing, otherwise this likely will never get exercised.
| context_segments = "" | ||
| target = context.target | ||
| namespace = _percent_encode(target.namespace) if target is not None else None | ||
| namespace_segment = f"/ns/{namespace}" if namespace else "" |
There was a problem hiding this comment.
While the S3 driver in Python drops the namespace segment, let's fix the GCS to be correct from the beginning by writing /ns/null, just like the other SDKs and drivers do.
This branch has not been deployed
3 participants
Add an experimental Google Cloud Storage driver so Temporal clients and workers can store large payloads in GCS and pass references through workflow history.
The driver follows the Python S3 driver's API, bucket selection, and object key layout. It uses the optional
gcloud-aio-storagelibrary for async I/O. The application owns and configures the storage client.Uploads use a single request, with resumable uploads disabled. Existing objects are left unchanged during concurrent uploads. Stored payloads include their metadata, and downloads verify SHA-256. The references can also be read by the Java GCS driver in temporalio/sdk-java#3151.
Includes setup documentation, Python tests, and a changelog entry.
Testing
poe lintpassed.gcloud-aio-storage9.6.4, the minimum supported version.Emulator tests require
STORAGE_EMULATOR_HOST. The real GCS and Java test harnesses are not committed.Related issue: #1502