feat: generate client, transport, and unit test templates for resumable uploads by parthea · Pull Request #18495 · googleapis/google-cloud-python

parthea · 2026-09-29T17:59:51Z

Add client, transport, and unit test template support for resumable upload RPCs.
Resumable upload methods accept an optional ResumableUploadConfig on the client and return ResumableUploadSession / AsyncResumableUploadSession from google.api_core.resumable_transfer. Since resumable uploads use HTTP, the gRPC and gRPC-async transports delegate resumable upload calls to the corresponding REST transport.
Removed compliance.proto from generated showcase goldens. See follow up issue Generated tests for showcase fail with compliance.proto #16312

gemini-code-assist

Code Review

This pull request adds support for resumable upload methods in the GAPIC generator, wrapping responses in ResumableUploadSession or AsyncResumableUploadSession and ensuring these methods route over REST even when gRPC transport is initialized. It updates templates, transports, and unit tests to support this protocol. The reviewer identified a critical issue in grpc_asyncio.py.j2 where AsyncRestTransport is instantiated on every call, which is highly inefficient and causes resource leaks; they suggested caching the transport instance instead.

daniel-sanche

It seems like there could be some runtime issues here. Can you make sure we have full coverage?

snippet-bot · 2026-09-30T15:56:10Z

No region tags are edited in this PR.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

Refresh this comment

ohmayr · 2026-09-30T21:23:51Z


        # Save the credentials.
+        {% if service.has_resumable_upload_methods %}
+        self._credentials: Any = credentials


nit: why are we declaring a type hint for a var and setting it to Any? is mypy complaining?

Can we omit it?

Yes, mypy complains when self._credentials (typed as Optional[google.auth.credentials.Credentials] ) is passed to
Async...RestTransport in grpc_asyncio.py (which expects Optional[google.auth.aio.credentials.Credentials]), due to the existing async
credential type-hint mismatch tracked in #16268:

I pushed a commit to show the mypy failure in b4b8c20. See build log https://github.com/googleapis/google-cloud-python/actions/runs/36883974364/job/110442747465?pr=18495. The specific failure is

nox > mypy -p google --check-untyped-defs google/showcase_v1beta1/services/resumable_upload_service/transports/grpc_asyncio.py:376: error: Argument "credentials" to "AsyncResumableUploadServiceRestTransport" has incompatible type "google.auth.credentials.Credentials | None"; expected "google.auth.aio.credentials.Credentials | None" [arg-type] Found 1 error in 1 file (checked 74 source files)

I found an existing workarounds here

google-cloud-python/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest_asyncio.py.j2

Lines 151 to 154 in 522db0e

{# Note: Type for creds is ignored because of incorrect type hint for creds in the client layer.

# TODO(https://github.com/googleapis/gapic-generator-python/issues/2177): Remove `# type: ignore` once

# we update the type hints for credentials to include asynchronous credentials in the client layer.

#}

google-cloud-python/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest_asyncio.py.j2

Lines 304 to 308 in 522db0e

{# Note: Type for creds is ignored because of incorrect type hint for creds in the client layer.

# TODO(https://github.com/googleapis/gapic-generator-python/issues/2177): Remove `# type: ignore` once

# we update the type hints for credentials to include asynchronous credentials in the client layer.

#}

credentials=self._credentials, # type: ignore

As far as I can tell, this is not a trivial fix. I'll add a comment with a link to the TODO comment to follow up.

Gemini highlighted a clash in GrpcAsyncIOTransport / AsyncRestTransport where grpc_helpers_async.create_channel takes sync credentials while AsyncRestTransport takes async credentials.

google-cloud-python/packages/google-api-core/google/api_core/grpc_helpers_async.py

Line 228 in 522db0e

credentials (google.auth.credentials.Credentials): The credentials. If

Done in 02bfacc

ohmayr · 2026-09-30T22:05:01Z

 from .grpc import {{ service.name }}GrpcTransport
+{% if service.has_resumable_upload_methods %}
+try:
+    from .rest_asyncio import Async{{ service.name }}RestTransport


What if it has resumable upload methods but does not have AsyncRestTransport? or have you eliminated this case with your changes to the generator.py?

I pushed a fix and updated test_get_response_resumable_upload_generates_async_client_and_rest_asyncio to ensure that at code generation time, generator.py will produce rest_asyncio.py (Async{{ service.name }}RestTransport), rest.py, and rest_base.py whenever service.has_resumable_upload_methods is True (even when rest_async_io_enabled is False or transport=grpc is passed).

At runtime, however, importing .rest_asyncio still raises ImportError if the library is installed without the optional [async_rest] (same as existing behavior for async rest SDKs)

See 43d0e42

ohmayr · 2026-09-30T22:10:00Z

        self._operations_client: Optional[operations_v1.OperationsClient] = None
        {% endif %}
+        {% if service.has_resumable_upload_methods %}
+        self._rest_transport: Optional[{{ service.name }}RestTransport] = None


it doesn't look clean that we are instantiating a rest transport within grpc code.

In another thread, you've mentioned that there is a strict requirement for grpc to have scotty methods (and I assume we need rest for that).

Maybe we should document that requirement.

Maybe there's a cleaner way to support this requirement. I haven't look into this in too much depth yet.

In another thread, you've mentioned that there is a strict requirement for grpc to have scotty methods (and I assume we need rest for that).

This is correct. We have a hard requirement as mentioned in #18495 (comment)

Maybe we should document that requirement.

I added a comment in grpc.py.j2 and grpc_asyncio.py.j2 documenting why REST is needed on gRPC transports in ce47c3c.

Maybe there's a cleaner way to support this requirement. I haven't look into this in too much depth yet.

Why delegation lives in the transport layer rather than the client layer:

{{ service.name }}Transport._prep_wrapped_messages() wraps self.{{ method }} for every method in service.methods during __init__. Implementing the method property on GrpcTransport and GrpcAsyncIOTransport satisfies the base Transport interface.

Client and AsyncClient remain transport-agnostic, dispatching all RPCs uniformly via self._transport._wrapped_methods[self._transport.<method>] and closing resources via self.transport.close().

ohmayr · 2026-09-30T22:12:21Z

+                self._stubs['{{ method.transport_safe_name|snake_case }}'] = _ErrorStub()
+            else:
+                if self._rest_transport is None:
+                    self._rest_transport = {{ service.name }}RestTransport(


what if rest.py file is missing? or can that not be the case?

Addressed in 43d0e42. rest.py won't be missing. See the test test_get_response_resumable_upload_generates_async_client_and_rest_asyncio

ohmayr · 2026-09-30T22:15:04Z

            {% endif %}
            """
+            {% if method.is_resumable_upload %}
+            http_options = _Base{{ service.name }}RestTransport._Base{{ method.name }}._get_http_options()


Can we use a macro for this base transport for sync and async code?

Good point. Addressed in 4588ebd. There is no change to the generated output

daniel-sanche

I wanted to bring all my remaining comments into one place, although I know some of these are already being addressed

daniel-sanche · 2026-10-02T18:10:17Z

Note: we'll have to bump this and the api_core floors for resumable uploads, after releasing #18542 and #18544

Fixed in 98b80fd. Test will fail until #18552 and #18551. To work around this, I'll temporarily install the versions from main, and then revert the temporary change once they're published.

The temporary change is in a07a705, which will be reverted once google-api-core and google-auth are released

daniel-sanche · 2026-10-02T18:44:43Z

+    config=config,
+    transport=self._session,
+    response_type={{ method.output.ident }},
+    start_retry=retry if isinstance(retry, {{ retry_class }}) else None,


Gemini pointed out that this will always be None, because the retry argument expected here is swallowed by _GapicCallable, so it never reaches here. It will always use the default retry

If we pass start_retry=retry in the client's rpc(...) call and accept start_retry on the transport method here, it will be passed through cleanly.

daniel-sanche · 2026-10-02T19:16:23Z

    {# TODO(https://github.com/googleapis/gapic-generator-python/issues/2121): Remove this condition when async rest is GA. #}
    {% if rest_async_io_enabled %}
    if HAS_ASYNC_REST_DEPENDENCIES:  # pragma: NO COVER
        _transport_registry["rest_asyncio"] = Async{{ service.name }}RestTransport


nit: This will add rest_asyncio to the transport registry, even if it is only used for resumable uploads. Most methods would raise NotImplementedErrors

Can we gate this, so it only shows up for libraries that fully enable rest_asyncio?

daniel-sanche · 2026-10-02T19:21:05Z

+                            transport._rest_transport = {{ service.name }}RestTransport(
+                                host=transport._host,
+                                credentials=transport._credentials,
+                                client_info=transport._client_info,


I think we'd need to pass down client_cert_source_for_mtls for mTLS support

(also for the async side, but it's not currently supported there. Maybe we should expose that limitation with an exception)

Are there other arguments we need to pass through?

daniel-sanche · 2026-10-02T19:32:28Z

+params = rest_helpers.flatten_query_params(query_params, strict=True)
+query_string = f"?{urllib.parse.urlencode(params)}" if params else ""
+upload_url = f"{self._host}{uri}{query_string}"
+headers: Dict[str, Any] = {**dict(metadata), **dict((config.headers or {}) if config else {})}


IIUC, should `"Content-Type": "application/json" be part of these headers as well?

daniel-sanche · 2026-10-02T19:52:38Z

+
+
+@dataclasses.dataclass
+class _AsyncResumableUploadSessionAdapter:


note: we can replace this with #18544

daniel-sanche · 2026-10-02T19:53:35Z

+                start_retry=retry if isinstance(retry, retries.AsyncRetry) else None,
+                **session_kwargs,
+            )
+            session.upload = functools.partial(session.upload, request_body=body or "")  # type: ignore[method-assign]


Note: we can remove this after integrating #18543

daniel-sanche · 2026-10-02T19:54:30Z

+                self._stubs['{{ method.transport_safe_name|snake_case }}'] = _ErrorStub()
+            elif HAS_ASYNC_REST:
+                # TODO(https://github.com/googleapis/google-cloud-python/issues/16268):
+                # Resolve the credential type mismatch between {{ service.grpc_asyncio_transport_name }}


note: the credential mismatch should go away with #18542

…loads

gemini-code-assist Bot reviewed Sep 29, 2026

View reviewed changes

Comment thread ...pic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc_asyncio.py.j2 Outdated

parthea changed the title ~~feat: add support for resumable uploads~~ feat: generate client, transport, and unit test templates for resumable uploads Sep 29, 2026

parthea marked this pull request as ready for review September 29, 2026 18:43

parthea requested a review from a team as a code owner September 29, 2026 18:43

parthea added this pull request to stack #18500 September 29, 2026 19:13

daniel-sanche requested changes Sep 29, 2026

View reviewed changes

parthea force-pushed the add-support-for-resumable-uploads-2 branch from b7f0140 to 5282533 Compare September 30, 2026 15:03

parthea force-pushed the add-support-for-resumable-uploads-2 branch from ce31648 to 2c60a8c Compare September 30, 2026 18:06

parthea removed this pull request from stack #18500 September 30, 2026 18:15

parthea changed the base branch from main to remove-compliance-proto September 30, 2026 18:16

parthea added this pull request to stack #18517 September 30, 2026 19:16

daniel-sanche reviewed Sep 30, 2026

View reviewed changes

Comment thread ...rator/gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc.py.j2 Outdated

parthea force-pushed the add-support-for-resumable-uploads-2 branch from a33d1fe to 389dbfb Compare September 30, 2026 20:23

Base automatically changed from remove-compliance-proto to main September 30, 2026 20:30

parthea force-pushed the add-support-for-resumable-uploads-2 branch from 1ddea43 to 8e785c8 Compare September 30, 2026 20:30

parthea assigned daniel-sanche Sep 30, 2026

parthea requested a review from daniel-sanche September 30, 2026 20:54

ohmayr reviewed Sep 30, 2026

View reviewed changes

daniel-sanche requested changes Oct 1, 2026

View reviewed changes

daniel-sanche reviewed Oct 1, 2026

View reviewed changes

Comment thread ...ges/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 Outdated

parthea force-pushed the add-support-for-resumable-uploads-2 branch from 39c26aa to f8fa453 Compare October 1, 2026 20:54

daniel-sanche reviewed Oct 1, 2026

View reviewed changes

Comment thread ...generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/_shared_macros.j2

Comment thread ...generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/_shared_macros.j2

daniel-sanche requested changes Oct 2, 2026

View reviewed changes

parthea added 4 commits October 2, 2026 20:25

feat: add support for resumable uploads

d9b6707

address review feedback

c448c74

update goldens

2d9db92

address review feedback

f3a255b

parthea added 22 commits October 2, 2026 20:25

update test

4405eec

address review feedback

52ef3ac

fix build

54fd19c

fix build

04b91b6

address review feedback

f712705

address review feedback

7ffd24b

update goldens

8059ad0

update goldens

0d9212e

address review feedback

f952d40

address review feedback

6424adc

address review feedback

08b304b

address review feedback

8684167

address review feedback

96f2f6b

address review feedback

76103bf

add test for bug where request body is not passed

1d6a0ff

fix: forward transcoded request body and query params in resumable up…

cc6b703

…loads

address review feedback

5352808

address review feedback

2303a13

address review feedback

8110282

fix build

9e41135

update unit test

4c9067a

fix build

7f0f479

parthea force-pushed the add-support-for-resumable-uploads-2 branch from d6b2985 to 7f0f479 Compare October 2, 2026 20:25

parthea added 2 commits October 2, 2026 20:32

address review feedback

98b80fd

sync minimum version of google-api-core for resumable uploads

ed5d9c9

parthea force-pushed the add-support-for-resumable-uploads-2 branch 3 times, most recently from aee4eef to e20d380 Compare October 3, 2026 00:53

temporary commit to be reverted

a07a705

parthea force-pushed the add-support-for-resumable-uploads-2 branch from e20d380 to a07a705 Compare October 3, 2026 00:56

	{# Note: Type for creds is ignored because of incorrect type hint for creds in the client layer.
	# TODO(https://github.com/googleapis/gapic-generator-python/issues/2177): Remove `# type: ignore` once
	# we update the type hints for credentials to include asynchronous credentials in the client layer.
	#}



		@dataclasses.dataclass
		class _AsyncResumableUploadSessionAdapter:

Conversation

parthea commented Sep 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

daniel-sanche left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

snippet-bot Bot commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

No region tags are edited in this PR.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daniel-sanche left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

parthea commented Sep 29, 2026 •

edited

Loading

snippet-bot Bot commented Sep 30, 2026 •

edited

Loading