-
Notifications
You must be signed in to change notification settings - Fork 37
grpc: Add interceptor API #484
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
base: grpc-common
Are you sure you want to change the base?
Conversation
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
796d4eb
to
52b0657
Compare
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 work! I have some comments nitpicking some details, mostly DSL, but everything else is well designed
and btw kDocs are really good there, quite easy to read and understand
grpc/grpc-core/src/commonMain/kotlin/kotlinx/rpc/grpc/internal/suspendClientCalls.kt
Show resolved
Hide resolved
grpc/grpc-core/src/commonMain/kotlin/kotlinx/rpc/grpc/internal/suspendServerCalls.kt
Outdated
Show resolved
Hide resolved
grpc/grpc-core/src/commonMain/kotlin/kotlinx/rpc/grpc/internal/suspendServerCalls.kt
Show resolved
Hide resolved
grpc/grpc-core/src/commonMain/kotlin/kotlinx/rpc/grpc/ManagedChannel.kt
Outdated
Show resolved
Hide resolved
grpc/grpc-core/src/commonMain/kotlin/kotlinx/rpc/grpc/ServerInterceptor.kt
Outdated
Show resolved
Hide resolved
grpc/grpc-core/src/commonTest/kotlin/kotlinx/rpc/grpc/test/BaseGrpcServiceTest.kt
Outdated
Show resolved
Hide resolved
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/internal/NativeClientCall.kt
Show resolved
Hide resolved
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/internal/NativeClientCall.kt
Show resolved
Hide resolved
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/internal/NativeClientCall.kt
Show resolved
Hide resolved
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
Signed-off-by: Johannes Zottele <[email protected]>
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, thank you!
|
||
|
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.
Please leave only one empty line in between declarations, same in some other places
* registerService(MyService()) | ||
* registerService(MyOtherService()) |
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.
This is a bit wrong example, should be:
registerService<MyService> { MyServiceImple() }
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.
Upsi
* registerService(MyService()) | ||
* registerService(MyOtherService()) | ||
* } |
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.
Same here
) { client, descriptor -> | ||
val response = client.clientStreamingRpc(descriptor, flow { | ||
repeat(5) { | ||
delay(100) |
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.
Please remove delay
from tests, replace with completable deferred
Will there be an interceptor feature in KRPC too? It would be really handy for the same use cases as presented here. |
@morki in some sort, yes, though later, we only maintain kRPC now, until we finish with gRPC (which is not much else to do, but still will take a couple of months) |
Subsystem
gRPC
Problem Description
A main feature of all gRPC libraries is their interceptor API.
We want a Kotlin idiomatic simple-to-use interceptor API (for client and server) that allows users to implement things like authentication, logging or tracing.
Solution
This PR adds a convenient Kotlin idiomatic interceptor API.