feat: add type annotations to wrapped grpc calls - #554
Conversation
Sorry, something went wrong.
|
@daniel-sanche Please could you address the lint failure? |
Sorry, something went wrong.
| # 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
Sorry, something went wrong.
|
|
||
|
|
||
| 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.
Sorry, something went wrong.
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
Sorry, something went wrong.
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Our gapic libraries declare that they return Iterable[SomeProto] or Awaitable[SomeProto] or Awaitable[AsyncIterable[SomeProto], but that doesn't tell the whole story. The objects returned are actually grpc.Call subclasses, which expose the ability to retrieve metadata from the rpc, or call cancel, or other trigger useful functionality.
Unfortunately, there is currently no way to use these grpc.Call methods without mypy errors, because of the restrictive return type annotation used
This 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 of Iterable[SomeProto], with all grpc.Call methods accessible
On the Async side, I made every _WrappedXYResponse class into a Generic container, and declared new GrpcAsyncStream[SomeProto] and AwaitableGrpcCall[SomeProto] types that can be used in place of AsyncIterable[SomeProto] and Awaitable[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