gRPC Service Improvements #106
Conversation
# Conflicts: # poetry.lock # pyproject.toml
cretz
force-pushed
the
grpc-updates
branch
6 times, most recently
from
8f6b1e5 to
f162dd1
Compare
| req: Vec<u8>, | ||
| retry: bool, | ||
| metadata: HashMap<String, String>, | ||
| timeout_millis: Option<u64>, |
There was a problem hiding this comment.
This could just be a duration directly, if pyo3 does that.
There was a problem hiding this comment.
It doesn't. Ref https://pyo3.rs/v0.16.4/conversions/tables.html. I could take a timedelta directly and convert myself, but this is close enough and ms is decent resolution (note I have done this with all other durations to Rust).
| @@ -73,7 +72,7 @@ async def connect( | |||
| ] = None, | |||
| tls: Union[bool, TLSConfig] = False, | |||
| retry_config: Optional[RetryConfig] = None, | |||
| static_headers: Mapping[str, str] = {}, | |||
| rpc_metadata: Mapping[str, str] = {}, | |||
There was a problem hiding this comment.
I think it makes sense to call this just "headers" still, it's probably a more familiar term for a lot of users.
There was a problem hiding this comment.
Don't want to do that as we also have Temporal headers and this is a high-level API. It is important to know this high-level construct maps to low-level RPC metadata. Note, in the low-level calls I do remove the rpc_ prefix. As for "headers" vs "metadata", this matches TypeScript and makes sense from a gRPC perspective.
| rpc_timeout: Optional RPC deadline to set for each RPC call. Note, | ||
| this is the timeout for each history RPC call not this overall | ||
| function. |
There was a problem hiding this comment.
People won't realize what "history" means in this context. I'd just say "...underlying RPC call...".
There was a problem hiding this comment.
Will change on next commit. I am a bit worried about confusion caused by this and if I should change to a fixed date time deadline, but I can wait until that is really needed.
What was changed
temporalio.workflow_servicepackage totemporalio.servicepackageClient.servicetoClient.workflow_service,Client.operator_service, andClient.test_serviceClient.service_clientwhich is just a combination of the three services and the raw connectionstatic_headerstorpc_metadataon client constructorrpc_metadatato every high-level and low-level gRPC callrpc_timeoutto every high-level and low-level gRPC callgrpcdependency completely optionalThe three backwards-incompatible changes are denoted with 💥. I know the review appears a bit daunting, but this is due to large refactoring I had to do. Externally it has minimal effect. The auto-generated proto files in
temporalio/apican be ignored.Checklist