Generate RPC calls in Bridge Client #1123

VegetarianOrc · 2025-09-25T21:27:30Z

What was changed

This PR adds code generation of the RPC call wrapper in the bridge client to ensure ease the maintenance of manually updating the methods when core has a proto update. This generation has been added to the end of the gen_protos_docker script.

Some changes were included to make the generation possible:

To eliminate a code generation path, the rpc_call and rpc_call_on_trait macros have been refactored to only include an exported macro named rpc_call that uses the fully qualified call syntax that was previously in the body of rpc_call_on_trait.
The multiple-pymethods feature of PyO3 was enabled to allow the ClientRef impl to be split into two modules.
The visibility of some elements of client module was elevated to pub(crate) for referencing from the client_rpc_generated module.

Why?

This allows us to easily keep the bridge client up to date with the RPC methods exposed via core.

Checklist

Closes [Bug] Batch operation feature in the Temporal Python SDK doesn't work #927
How was this tested:

The generated client was tested against the existing test suite and used to ensure the repro described in #927 correctly functions against a Temporal dev server.

Any docs updates needed?

CLAassistant · 2025-09-25T21:27:37Z

All committers have signed the CLA.

cretz · 2025-09-26T13:40:28Z

scripts/gen_bridge_client.py

I think we might also want to generate all of that mess in temporalio/service.py. Off the top of my head, what I would suggest is to generate some file at temporalio/bridge/client_generated.py.

Could do the simple way of just having an init_workflow_service_client_calls(client: ServiceClient) that just does what it does today which is makes a _new_call for each known service name. Alternatively, a cleaner approach may be to just move the WorkflowService (and operator and cloud equivalents) to this generated file and have explicit methods created for each call that do as expected and then alias the service class publicly (or have the public WorkflowService class extend the internal/bridge form).

cretz

LGTM overall, just little things

temporalio/bridge/src/client_rpc_generated.rs

cretz · 2025-09-29T22:02:49Z

temporalio/service.py

 import temporalio.api.workflowservice.v1
 import temporalio.bridge.client
 import temporalio.bridge.proto.health.v1
+import temporalio.bridge.services_generated as svc_gen


Suggested change

import temporalio.bridge.services_generated as svc_gen

import temporalio.bridge.services_generated

Pedantic and not technically required, but usually we fully qualify each use when referring to ourselves in these files

Updated these to be fully qualified. I also moved the generated python to be in bridge/generated so I could add a pydocstyle exclusion for the file. If that doesn't seem reasonable, I can see about spitting out some autogen'd doc strings.

I do think that' be nice. Yeah, one issue about moving the base class into the bridge area is that it won't be as clear on https://python.temporal.io (we consider the bridge private code and don't show it in API docs). This will be the first time user-facing code references the bridge module (in this case for a class extension). But I don't think it's that big of a deal.

Not huge, but I don't see any good reason to move the code to bridge rather than another generated file. Some autgenerated doc strings seem fine.

tests/test_service.py

cretz

Nothing blocking, though we may want to consider not putting user-facing components in internal bridge module. @tconley1428 - thoughts?

…ying on previous behavior but still contain the same assertions

…port in service.py

…s to a generated directory to allow pydocstyle exclusion. Add linter ignore rule on tests/worker/test_workflow.py:4840 to avoid a linter rule about method assignment.

tconley1428

I think we should consider

tconley1428 · 2025-09-25T23:04:00Z

scripts/gen_bridge_client.py

+    file_descriptors: list[FileDescriptor],
+    output_file: str = "temporalio/bridge/src/client_rpc_generated.rs",
+):
+    print("generating bridge rpc calls")


I think maybe one print for the whole thing, but I'm not sure we need all the steps. That said, marginal

tconley1428 · 2025-09-25T23:05:01Z

temporalio/bridge/Cargo.toml

 futures = "0.3"
 prost = "0.13"
 pyo3 = { version = "0.25", features = [
    "extension-module",


No good place to put this comment, but I think we should consider adding a test for the actual user issue, just to have the coverage we fixed it.

I've added a test that executes all the possible rpc calls and ensures that none result in the "Unknown RPC call" exception. The calls are made with empty request objects so exceptions are expected, but the absence of the "Unknown RPC call" exception should indicate that all the rpc calls are wired up from Python -> Bridge -> Core. I confirmed that removing one of the RPC calls from the generated Rust source produces a test failure which was the cause of the user issue.

https://github.com/temporalio/sdk-python/pull/1123/files#diff-d7fb70d71c01e6c6a932249a22ad45e9e8e5dd800f8d9260922b7726ada72402R170-R238

Also open to other approaches on getting the coverage!

tconley1428 · 2025-09-30T14:22:02Z

temporalio/service.py

 import temporalio.api.workflowservice.v1
 import temporalio.bridge.client
 import temporalio.bridge.proto.health.v1
+import temporalio.bridge.services_generated as svc_gen


Not huge, but I don't see any good reason to move the code to bridge rather than another generated file. Some autgenerated doc strings seem fine.

…on is wired up through bridge and core

… call'

VegetarianOrc · 2025-10-02T20:33:05Z

pyproject.toml

+  "httpx>=0.28.1",
+  "pytest-pretty>=1.3.0",
+  "openai-agents[litellm]>=0.3,<0.4",
+  "googleapis-common-protos==1.70.0",


@tconley1428 , want to confirm with you that adding this googleapis-common-protos to this dev dependecy list won't cause any adverse effects. The new test that imports to proto descriptors relies on it, but if it's not reasonable to add, I'll modify the test to take a different approach.

I don't think there's any adverse impact. It would only affect folks working on the repo, not library users. If it is needed to improve the test coverage, worth it I'd say.

tconley1428 · 2025-10-03T20:51:03Z

tests/test_service.py

+        rpc_call = getattr(target_service, method_name)
+        try:
+            await rpc_call(request, timeout=timedelta(milliseconds=1))
+        except Exception as err:


Can we take a quick look that the exceptions which do occur are all after the call is made? If any of them fail with like, parameter checking or something before that, then they wouldn't be covered.

The potential failures happen in the _BridgeServiceClient._rpc_call where there are 3 outcomes leading to exceptions that I'm aware of.

The call to connect the rpc client fails. The connect method calls into the bridge source where it potentially raises a RuntimeError

The rpc call gets rejected by bridge before actually being dispatched due to the unknown RPC call error which is raised as a ValueError in the generated service clients on the Rust side.

The rpc call is proxied through bridge and results in a Bridge RPCError which gets caught and re-raised as a python RPCError by the _BridgeServiceClient.

With those listed out, I've updated the test to explicitly fail on the ValueError and explicitly pass on the RPCError which is what is returned when the call is proxied successfully but is rejected by the server.

I've left the RuntimeError unhandled as the test should have a client that's able to connect based on the existing test structure.

…eError

cretz · 2025-10-06T14:14:22Z

pyproject.toml

 gen-protos = "uv run scripts/gen_protos.py"
 gen-protos-docker = "uv run scripts/gen_protos_docker.py"


@VegetarianOrc - to confirm, we decided to move the simple uv-run type of steps out of gen_protos_docker any as poe sub-commands to these two commands right?

VegetarianOrc requested a review from a team as a code owner September 25, 2025 21:27

cretz reviewed Sep 26, 2025

View reviewed changes

cretz reviewed Sep 29, 2025

View reviewed changes

cretz approved these changes Sep 30, 2025

View reviewed changes

VegetarianOrc added 7 commits September 30, 2025 10:59

Add initial draft of rpc call generation

168aa7c

remove test log statement. Reorder gen_bridge_client.py

8ac8f31

Add generation of python services. Update a couple tests to avoid rel…

3e01e9b

…ying on previous behavior but still contain the same assertions

remove debug prints

f43225a

run format after generating files

77c4dd8

Remove debug print. Remove unused imports. Fully qualify generated im…

d34886b

…port in service.py

add call to cargo fmt after generation. Move generated python service…

2812a80

…s to a generated directory to allow pydocstyle exclusion. Add linter ignore rule on tests/worker/test_workflow.py:4840 to avoid a linter rule about method assignment.

VegetarianOrc force-pushed the bridge/generate_rpc_calls branch from a971dcf to 2812a80 Compare September 30, 2025 17:59

tconley1428 reviewed Sep 30, 2025

View reviewed changes

VegetarianOrc and others added 6 commits October 1, 2025 11:33

Remove some prints. Add simple test that confirms start_batch_operati…

e427c15

…on is wired up through bridge and core

remove simple test in favor of a more thorough check for 'unknown rpc…

888e1f8

… call'

Add generated doc strings and remove exception from pydocstyle

bb4dca4

Merge branch 'main' into bridge/generate_rpc_calls

8a35b88

Try out reference descriptors directly rather than using importlib

257395c

Add googleapis-common-protos to dev dependencies.

aa7c20e

VegetarianOrc commented Oct 2, 2025

View reviewed changes

Merge branch 'main' into bridge/generate_rpc_calls

b6ac958

tconley1428 reviewed Oct 3, 2025

View reviewed changes

Update test to explicitly pass on RPCError and explictly fail on Valu…

764cc3e

…eError

cretz reviewed Oct 6, 2025

View reviewed changes

tconley1428 approved these changes Oct 6, 2025

View reviewed changes

VegetarianOrc merged commit 26e2e61 into main Oct 6, 2025
26 of 27 checks passed

VegetarianOrc deleted the bridge/generate_rpc_calls branch October 6, 2025 19:45

VegetarianOrc mentioned this pull request Oct 6, 2025

Add googleapis-common-protos to fix tests in build-binaries workflow #1143

Merged

	import temporalio.bridge.services_generated as svc_gen
	import temporalio.bridge.services_generated

		gen-protos = "uv run scripts/gen_protos.py"
		gen-protos-docker = "uv run scripts/gen_protos_docker.py"

Generate RPC calls in Bridge Client #1123

Generate RPC calls in Bridge Client #1123

Uh oh!

Conversation

VegetarianOrc commented Sep 25, 2025

What was changed

Why?

Checklist

Uh oh!

CLAassistant commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cretz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cretz Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cretz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tconley1428 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!

VegetarianOrc Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

CLAassistant commented Sep 25, 2025 •

edited

Loading

cretz Sep 30, 2025 •

edited

Loading

cretz left a comment •

edited

Loading

VegetarianOrc Oct 2, 2025 •

edited

Loading