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

Use wire Grpc client instead of misks #954

Merged
merged 1 commit into from May 2, 2019

Conversation

Projects
None yet
4 participants
@Synesso
Copy link
Collaborator

commented May 1, 2019

No description provided.

@mightyguava

This comment has been minimized.

Copy link
Collaborator

commented May 1, 2019

Yay grpc! Are there docs for this client? Is this the code? https://github.com/square/wire/tree/master/wire-grpc-client. How does it relate to https://github.com/grpc/grpc-java?

@Synesso

This comment has been minimized.

Copy link
Collaborator Author

commented May 1, 2019

Yay grpc! Are there docs for this client? Is this the code? https://github.com/square/wire/tree/master/wire-grpc-client.

That looks like the one. Any docs available @oldergod ?

How does it relate to https://github.com/grpc/grpc-java?

AFAIK (which isn't much, admittedly), there's no connection.

@mightyguava

This comment has been minimized.

Copy link
Collaborator

commented May 1, 2019

Hmm it doesn't look like a full grpc client impl with all the goodies: https://github.com/grpc/grpc/tree/master/doc. Are we rebuilding a subset of grpc features in this client?

@Synesso

This comment has been minimized.

Copy link
Collaborator Author

commented May 1, 2019

@mightyguava I guess that's another question for @oldergod / wire-grpc-client.

@oldergod
Copy link
Member

left a comment

There is no doc right now; if the language target is Kotlin, services will also be included in the generated code.

it doesn't look like a full grpc client impl with all the goodies

We intend to generate code for services from a server point of view; what else do you need?

import kotlinx.coroutines.channels.ReceiveChannel
import kotlinx.coroutines.channels.SendChannel

// TODO(jwilson): generate this and remove it from git.

This comment has been minimized.

Copy link
@oldergod

oldergod May 1, 2019

Member

This is already generated code though?

This comment has been minimized.

Copy link
@swankjesse

swankjesse May 2, 2019

Member

Wasn't getting emitted for us. Maybe fixed beyond alpha01?

): GrpcClient {
return GrpcClient.Builder()
.client(client)
.baseUrl(url.toString()) // TODO(jwilson): this should also take a URL. Come on guys.

This comment has been minimized.

Copy link
@oldergod
@mightyguava

This comment has been minimized.

Copy link
Collaborator

commented May 1, 2019

We intend to generate code for services from a server point of view; what else do you need?

I can't claim to know what we need in a grpc client. Haven't personally used it in a high traffic setting like square. But grpc has more stuff than just the wire protocol.

From the FAQ, it has:

  • interaction with flow-control at the application layer
  • cascading call-cancellation (this is super cool)
  • client-side load balancing & failover (this is kinda annoying actually, but they make it out to be something that becomes more useful at larger scale?)

It should also be really stable now since it's been available for several years. We probably don't actually need all of these features, but I'd love to understand why we want our own implementation of the client, versus just using the official implementation.

I really like that the Kotlin/wire for protos and grpc is much cleaner with wire-grpc-client, but that seems like something we could build as an adapter, rather than a standalone re-implementation.

@swankjesse

This comment has been minimized.

Copy link
Member

commented May 2, 2019

My plan is to do gRPC on the same rails as our web APIs. Same TLS, tracing, metrics, auth, config, load balancing, routing, ... everything. The only thing different between web and gRPC is that gRPC specifies schema & action names in .proto.

If we use Google’s gRPC stack we can’t share much with the web.

import kotlinx.coroutines.channels.ReceiveChannel
import kotlinx.coroutines.channels.SendChannel

// TODO(jwilson): generate this and remove it from git.

This comment has been minimized.

Copy link
@swankjesse

swankjesse May 2, 2019

Member

Wasn't getting emitted for us. Maybe fixed beyond alpha01?

@swankjesse swankjesse merged commit 255d803 into master May 2, 2019

2 checks passed

ci/circleci: java Your tests passed on CircleCI!
Details
ci/circleci: node Your tests passed on CircleCI!
Details

@oldergod oldergod deleted the jesse+jem/use-wires-grpcclient-instead-of-misks branch May 2, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.