-
Notifications
You must be signed in to change notification settings - Fork 435
Require generated client protocols to accept CallOptions
#865
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
Conversation
|
Note: 3c6fe84 contains the actual change and a regenerated Echo client. The other commits are just other clients regenerated. |
Lukasa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, I generally think this looks good. I've left a few small notes in the diff: nothing major, just some cleanups and thoughts.
0fa5d9d to
c310e5c
Compare
Motivation: The generated protocol for clients allows the caller to optionally pass in some `CallOptions`. The generated client conforming to this protocol defaults this argument to `nil` and falls back to default options set on that client in that case. It is therefore possible to call, for example, `theClient.someUnaryRPC(someRequest)` but not `theProtocol.someUnaryRPC(someRequest)`. Without explicitly passing the call options, the protocol offers little value when coding to the protocol rather than to a concrete implementation. Modifications: - Generated client protocols now extend `GRPCClient` (previously the generated client implementations conformed to `GRPCClient` and the protocol) - Generated client protocol requires `CallOptions` - Generated client protocols now have a generated extension for each RPC which does not require call options whose implementation forwards the default call options courtesy of `GRPCClient`. Result: - It is possible to reasonably implement code against the generated protocol rather than a concrete implementation - We lose the ability to call an RPC with `callOptions: nil`
c310e5c to
aa11601
Compare
Lukasa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go for it.
Motivation: When consuming gRPC it is often helpful to be able to write tests that ensure the client is integrated correctly. At the moment this is only possible by running a local gRPC server with a custom service handler to return the responses you would like to test. Modifications: This builds on work in grpc#855, grpc#864, and grpc#865. This pull request introduces code generation for the test clients. It provides type-safe wrappers and convenience methods on top of `FakeChannel` and a code-gen option to enable 'TestClient' generation. It also removes an `init` requirement on the `GRPCClient` protocol. Result: Users can generate test clients.
Motivation: When consuming gRPC it is often helpful to be able to write tests that ensure the client is integrated correctly. At the moment this is only possible by running a local gRPC server with a custom service handler to return the responses you would like to test. Modifications: This builds on work in grpc#855, grpc#864, and grpc#865. This pull request introduces code generation for the test clients. It provides type-safe wrappers and convenience methods on top of `FakeChannel` and a code-gen option to enable 'TestClient' generation. It also removes an `init` requirement on the `GRPCClient` protocol. Result: Users can generate test clients.
Motivation: When consuming gRPC it is often helpful to be able to write tests that ensure the client is integrated correctly. At the moment this is only possible by running a local gRPC server with a custom service handler to return the responses you would like to test. Modifications: This builds on work in grpc#855, grpc#864, and grpc#865. This pull request introduces code generation for the test clients. It provides type-safe wrappers and convenience methods on top of `FakeChannel` and a code-gen option to enable 'TestClient' generation. It also removes an `init` requirement on the `GRPCClient` protocol. Result: Users can generate test clients.
* Provide the codegen with an option to generate test clients Motivation: When consuming gRPC it is often helpful to be able to write tests that ensure the client is integrated correctly. At the moment this is only possible by running a local gRPC server with a custom service handler to return the responses you would like to test. Modifications: This builds on work in #855, #864, and #865. This pull request introduces code generation for the test clients. It provides type-safe wrappers and convenience methods on top of `FakeChannel` and a code-gen option to enable 'TestClient' generation. It also removes an `init` requirement on the `GRPCClient` protocol. Result: Users can generate test clients. * Regenerate * fix type, add assertion * Add "Remaining"
Motivation:
The generated protocol for clients allows the caller to optionally pass
in some
CallOptions. The generated client conforming to this protocoldefaults this argument to
niland falls back to default options set onthat client in that case.
It is therefore possible to call, for example,
theClient.someUnaryRPC(someRequest)but nottheProtocol.someUnaryRPC(someRequest).Without explicitly passing the call options, the protocol offers little
value when coding to the protocol rather than to a concrete
implementation.
Modifications:
GRPCClient(previously thegenerated client implementations conformed to
GRPCClientand theprotocol)
CallOptionswhich does not require call options whose implementation forwards the
default call options courtesy of
GRPCClient.Result:
protocol rather than a concrete implementation
callOptions: nil