Abstract Channel Initialization behind Client #21
Conversation
|
|
||
| class ClientOptions(TypedDict, total=False): | ||
| domain: str | ||
| target: str |
There was a problem hiding this comment.
nit: service_endpoint? target is too general, I guess this is for socket address or a yarpc dispatcher for internal services. How are we going to integrate this internally?
There was a problem hiding this comment.
Yeah, target is rather weird. I picked this name because it matches the naming from the GRPC api: https://grpc.github.io/grpc/python/grpc_asyncio.html#grpc.aio.secure_channel .
In the Java client we specify host and port, I don't know the equivalent in Go.
Internally I think this will work similar to Java, where we use the GRPC transport.
There was a problem hiding this comment.
How about adding some comment on this or maybe change to grpc_target then?
| async def close(self) -> None: | ||
| await self._channel.close() | ||
|
|
||
| def _validate_and_copy_defaults(options: ClientOptions) -> ClientOptions: |
There was a problem hiding this comment.
nit: move to ClientOptions class method so it's less likely someone implements something similar in other packages
| await self._channel.close() | ||
|
|
||
| def _validate_and_copy_defaults(options: ClientOptions) -> ClientOptions: | ||
| if "target" not in options: |
There was a problem hiding this comment.
maybe options.target is not None so it's discoverable on how this field is used through IDE?
Our server being yarpc means that there are mandatory headers needed on every request. To include these we need to add mandatory interceptors. For additional things like retries and error mapping we'll also want additional interceptors. GRPC's sync implementation allows for adding interceptors to an existing channel, while the async implementation does not. As a result, our client needs to be responsible for Channel creation. Add GRPC channel options to ClientOptions and create the Channel within Client. This approach largely matches how the Java client approaches it, although it does allow for overriding the Channel still.
Our server being yarpc means that there are mandatory headers needed on every request. To include these we need to add mandatory interceptors. For additional things like retries and error mapping we'll also want additional interceptors.
GRPC's sync implementation allows for adding interceptors to an existing channel, while the async implementation does not. As a result, our client needs to be responsible for Channel creation.
Add GRPC channel options to ClientOptions and create the Channel within Client. This approach largely matches how the Java client approaches it, although it does allow for overriding the Channel still.
What changed?
Why?
How did you test it?
Potential risks
Release notes
Documentation Changes