-
Notifications
You must be signed in to change notification settings - Fork 40
grpc-native: Support client calls on native #452
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
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.
Wow, great work! Thanks for the amount of comments and the big test set, it was very pleasant to read!
I have some comments and suggestions below, please address them before merging.
One big question that I have, is using C lib instead of C++ one - how hard it would be for use to support gRPC features as Auth, interceptors, etc? It is available is C API?
grpc/grpc-core/src/commonTest/kotlin/kotlinx/rpc/grpc/test/RawClientTest.kt
Outdated
Show resolved
Hide resolved
grpc/grpc-core/src/commonTest/kotlin/kotlinx/rpc/grpc/test/RawClientTest.kt
Show resolved
Hide resolved
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/internal/CallbackFuture.kt
Outdated
Show resolved
Hide resolved
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/internal/CompletionQueue.kt
Outdated
Show resolved
Hide resolved
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/internal/CompletionQueue.kt
Outdated
Show resolved
Hide resolved
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/internal/NativeManagedChannel.kt
Outdated
Show resolved
Hide resolved
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/internal/utils.kt
Outdated
Show resolved
Hide resolved
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/ManagedChannel.native.kt
Show resolved
Hide resolved
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.
LGTM!
Please look at the comments below
Also, please revisit all places where you do check
and replace with internalError
if needed, I saw at least one such place
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/internal/CompletionQueue.kt
Show resolved
Hide resolved
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/internal/CompletionQueue.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/ManagedChannel.native.kt
Outdated
Show resolved
Hide resolved
grpc/grpc-core/src/nativeMain/kotlin/kotlinx/rpc/grpc/ManagedChannel.native.kt
Show resolved
Hide resolved
Signed-off-by: Johannes Zottele <official@johannes-zottele.at>
Signed-off-by: Johannes Zottele <official@johannes-zottele.at>
Signed-off-by: Johannes Zottele <official@johannes-zottele.at>
Signed-off-by: Johannes Zottele <official@johannes-zottele.at>
Signed-off-by: Johannes Zottele <official@johannes-zottele.at>
Signed-off-by: Johannes Zottele <official@johannes-zottele.at>
Signed-off-by: Johannes Zottele <official@johannes-zottele.at>
Signed-off-by: Johannes Zottele <official@johannes-zottele.at>
Signed-off-by: Johannes Zottele <official@johannes-zottele.at>
Signed-off-by: Johannes Zottele <official@johannes-zottele.at>
Signed-off-by: Johannes Zottele <official@johannes-zottele.at>
Signed-off-by: Johannes Zottele <official@johannes-zottele.at>
Signed-off-by: Johannes Zottele <official@johannes-zottele.at>
Signed-off-by: Johannes Zottele <official@johannes-zottele.at>
c5fd51e
to
10f4c7b
Compare
* grpc-native: Initial client setup Signed-off-by: Johannes Zottele <official@johannes-zottele.at> * grpc-native: Working CompletionQueue Signed-off-by: Johannes Zottele <official@johannes-zottele.at> * grpc-native: Working version of NativeClientCall Signed-off-by: Johannes Zottele <official@johannes-zottele.at> * grpc-native: Working rewrite state Signed-off-by: Johannes Zottele <official@johannes-zottele.at> * grpc-native: Add callback future Signed-off-by: Johannes Zottele <official@johannes-zottele.at> * grpc-native: Refactor to use callbacks instead of coroutines Signed-off-by: Johannes Zottele <official@johannes-zottele.at> * grpc-native: Fixes after rebase Signed-off-by: Johannes Zottele <official@johannes-zottele.at> * grpc-native: Implement bridge to common Signed-off-by: Johannes Zottele <official@johannes-zottele.at> * grpc-native: Remove unnecessary C header definitions Signed-off-by: Johannes Zottele <official@johannes-zottele.at> * grpc-native: Rename grpcpp_c to kgrpc Signed-off-by: Johannes Zottele <official@johannes-zottele.at> * grpc-native: Reduce library dependencies (fixes KRPC-185) Signed-off-by: Johannes Zottele <official@johannes-zottele.at> * grpc-native: Write docs Signed-off-by: Johannes Zottele <official@johannes-zottele.at> * grpc-native: Address PR comments Signed-off-by: Johannes Zottele <official@johannes-zottele.at> * grpc-native: Address PR comments Signed-off-by: Johannes Zottele <official@johannes-zottele.at> --------- Signed-off-by: Johannes Zottele <official@johannes-zottele.at>
Subsystem
gRPC/Native
Solution
This PR uses the grpc-core (C) library to support client calls for Kotin/Native.