-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
glide.yaml: update grpc #9697
glide.yaml: update grpc #9697
Conversation
LGTM, but let's line this up into the queue of "potentially critical PRs" and roll it out alone. |
@petermattis we need at least a tiny bit of infrastructure for these PRs. Should we create a label to that end? |
Agree that we should sequence the merging of this PR. A label sounds good. Care to bikeshed a name? |
Let's use |
LGTM |
Updated. |
This update includes 118 commits. grpc/grpc-go@79b7c34...777daa1 Interesting PRs: - Client side interceptors: grpc/grpc-go#867 - Stricter error handling: grpc/grpc-go#876 - More robust cancellation: grpc/grpc-go#882 - Fix context leak: grpc/grpc-go#891 - Client load balancing: grpc/grpc-go#899 - Fix quota miscounting: grpc/grpc-go#947 Also updates gogo/protobuf to pick up gogo/protobuf@8d70fb3 which is required due to the `grpc.SupportPackageIsVersion` mechanism.
beta is cut, merging. |
Review status: 0 of 13 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful. pkg/kv/transport.go, line 166 at r1 (raw file):
Somehow this part of the change escaped my notice. Seems like purely cleanup that is unrelated to the grpc upgrade. I'm not sure if this is more readable. If you wanted to avoid the multiple Comments from Reviewable |
Would you like me to send a follow up?
…On Dec 20, 2016 10:48 AM, "Peter Mattis" ***@***.***> wrote:
Review status: 0 of 13 files reviewed at latest revision, 1 unresolved
discussion, all commit checks successful.
------------------------------
*pkg/kv/transport.go, line 166 at r1
<https://reviewable.io:443/reviews/cockroachdb/cockroach/9697#-KZRpTtK4luhkcoo0yBb:-KZRpTtK4luhkcoo0yBc:b-s8tezy>
(raw file
<https://github.com/cockroachdb/cockroach/blob/cf371771bbff0de57b8776973d76a6aad6426f39/pkg/kv/transport.go#L166>):*
gt.setPending(client.args.Replica, true)
batchFn := func(ctx context.Context, args *roachpb.BatchRequest) (*roachpb.BatchResponse, error) {
Somehow this part of the change escaped my notice. Seems like purely
cleanup that is unrelated to the grpc upgrade. I'm not sure if this is more
readable. If you wanted to avoid the multiple go calls, we could have
moved the localServer lookup inside of the single go call.
------------------------------
*Comments from Reviewable
<https://reviewable.io:443/reviews/cockroachdb/cockroach/9697>*
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#9697 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABdsPOnyS45I1Qc-KKa13mNfDR4lrC83ks5rJ_iqgaJpZM4KNKmY>
.
|
Yes. |
This update includes 118 commits.
grpc/grpc-go@79b7c34...777daa1
Interesting PRs:
Support client side interceptor grpc/grpc-go#867
Close headerChan if processHeaderField sets error grpc/grpc-go#876
Move balancer initialization into a goroutine grpc/grpc-go#882
Use user context instead of creating new context for client side stream grpc/grpc-go#891
Add the basic support of grpclb grpc/grpc-go#899
Use the correct error in defer call grpc/grpc-go#947
Also updates gogo/protobuf to pick up
gogo/protobuf@8d70fb3 which is required due
to the
grpc.SupportPackageIsVersion
mechanism.This change is