Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metadata on ClientCallDetails of async interceptor tuple instead of Metadata object #34298

Closed
fdellekart opened this issue Sep 10, 2023 · 1 comment · Fixed by #34347
Closed

Comments

@fdellekart
Copy link

What version of gRPC and what language are you using?

gRPC 1.56.2
Python 3.11

What operating system (Linux, Windows,...) and version?

Ubuntu 22.04

What runtime / compiler are you using (e.g. python version or version of gcc)

Python 3.11

What did you do?

I am writing an async client side interceptor, which adds some metadata to requests before they are sent to the server.
Inside of this interceptor I used the add method of the metadata object on the client_call_details.

from grpc.aio import UnaryUnaryClientInterceptor

class Interceptor(UnaryUnaryClientInterceptor):
    async def intercept_unary_unary(
        self,
        continuation: Callable[[ClientCallDetails, Message], UnaryUnaryCall],
        client_call_details: ClientCallDetails,
        request: Message,
    ) -> Union[UnaryUnaryCall, Message]:
            client_call_details.metadata.add("my-key", "my-value")
            response = await continuation(client_call_details, request)
            return response

When calling the RPC on the corresponding client stub I passed the metadata as a Sequence[Tuple[str, str]], as this is how it works for the synchronous version.

async with grpc.aio.insecure_channel(f"localhost:{AIO_PORT}") as channel:
    stub = gRPCTestServiceStub(channel)
    await stub.TestServe(gRPCTestMessage(text="test"), metadata=(("my-call-md", "my-call-md-value"),))

What did you expect to see? / What did you see instead?

This then led to AttributeError: 'tuple' object has no attribute 'add' because the tuple is passed on to the interceptor instead of a grpc.aio.Metadata object.
I understand that this is more of a wrong usage from my side as I should use the metadata object instead of a tuple, however, I think the general behavior could be improved to avoid confusion. IMO it is very likely for this to happen because of the corresponding call pattern on the synchronous version and without digging into the library code it was not obvious to me that in the async version the metadata should be passed differently.

IMO, as grpc.aio.Metadata even already has a method to do this, if a tuple is passed as call metadata it could be transformed into the object of the correct class. If not that, then maybe at least raise an understandable exception.

fdellekart added a commit to fdellekart/sentry-python that referenced this issue Sep 13, 2023
Opened an issue in grpc repo to ask if this is inteded behaviour
If it should be changed one day the the .add method of the metadata
object would avoid reconstructing the whole client call details.

Link to issue:
grpc/grpc#34298
@XuanWang-Amos
Copy link
Contributor

XuanWang-Amos commented Sep 14, 2023

Thanks for raising this issue, we agree that it's not ideal to raise AttributeError in this case, since we specifically said it should be a Metadata object in our API:

metadata: Optional[Metadata]

It's better to convert metadata from Tuple to grpc.aio.Metadata, I'll create a PR for this change shortly.

XuanWang-Amos added a commit that referenced this issue Oct 19, 2023
Fix: #34298


<!--

If you know who should review your pull request, please assign it to
that
person, otherwise the pull request would get assigned randomly.

If your pull request is for a specific language, please add the
appropriate
lang label.

-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants