feat: add type annotations to wrapped grpc calls #554
Conversation
product-auto-label
bot
added
size: m
|
@daniel-sanche Please could you address the lint failure? |
|
@parthea done |
| # public type alias denoting the return type of streaming gapic calls | ||
| GrpcAsyncStream = _WrappedStreamResponseMixin[S] | ||
| # public type alias denoting the return type of unary gapic calls | ||
| AwaitableGrpcCall = _WrappedUnaryResponseMixin[U] |
There was a problem hiding this comment.
Let me know if you have other naming suggestions for these (AsyncGrpcCall? GrpcAsyncIterable?)
I liked Awaitable because it's clear how to interact with it, and Stream instead of Iterable because it can do more than just iterate. But names are hard and I'm open to alternatives
google/api_core/grpc_helpers.py
Outdated
| @@ -79,7 +83,7 @@ def error_remapped_callable(*args, **kwargs): | |||
| return error_remapped_callable | |||
|
|
|||
|
|
|||
| class _StreamingResponseIterator(grpc.Call): | |||
| class GrpcStream(grpc.Call, Generic[S]): | |||
There was a problem hiding this comment.
Do you think this could be a breaking change? Could we keep the name as _StreamingResponseIterator ? There are many hits for _StreamingResponseIterator in Google search. I'm worried that changing _StreamingResponseIterator could cause issues downstream in user code as it is a response we provide.
There was a problem hiding this comment.
Makes sense, I'll create a type alias instead, like I did on the async side
I thought it would be safe to change since it's a private class, but better to be on the safe side
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Our gapic libraries declare that they return
Iterable[SomeProto]orAwaitable[SomeProto]orAwaitable[AsyncIterable[SomeProto], but that doesn't tell the whole story. The objects returned are actuallygrpc.Callsubclasses, which expose the ability to retrieve metadata from the rpc, or callcancel, or other trigger useful functionality.Unfortunately, there is currently no way to use these
grpc.Callmethods without mypy errors, because of the restrictive return type annotation usedThis PR is the first step in addressing this, by giving us a more powerful return type in
api-core.On the sync side, I made _StreamingResponseIterator into a Generic container, and gave it a better public facing name. This way, we we can return
GrpcStream[SomeProto]instead ofIterable[SomeProto], with all grpc.Call methods accessibleOn the Async side, I made every
_WrappedXYResponseclass into a Generic container, and declared newGrpcAsyncStream[SomeProto]andAwaitableGrpcCall[SomeProto]types that can be used in place ofAsyncIterable[SomeProto]andAwaitable[SomeProto]respectively.For more context on the motivating problem, see googleapis/gapic-generator-python#1856. This PR lays some of the groundwork for resolving that issue in the future. But the type improvements here should also stand alone