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

bump(google.golang.org/grpc): 708a7f9f3283aa2d4f6132d287d78683babe55c8 #18071

Closed
wants to merge 1 commit into from

Conversation

soltysh
Copy link
Member

@soltysh soltysh commented Jan 11, 2018

@mfojtik @smarterclayton the bump looks reasonable, although I'm yet to carefully go through every change. Let's see how our tests infra likes it.

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: soltysh
We suggest the following additional approver: pweil-

Assign the PR to them by writing /assign @pweil- in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot added the vendor-update Touching vendor dir or related files label Jan 11, 2018
@openshift-ci-robot openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jan 11, 2018
@bparees bparees removed their request for review January 11, 2018 16:22
@smarterclayton
Copy link
Contributor

What commits were there after this to the next major release of gRPC? Any risk of this introducing breaks fixed immediately after this commit?

@smarterclayton
Copy link
Contributor

/test all

@soltysh
Copy link
Member Author

soltysh commented Jan 11, 2018

What commits were there after this to the next major release of gRPC? Any risk of this introducing breaks fixed immediately after this commit?

@smarterclayton the big changes are: adding stats (grpc/grpc-go#961) and server list expiration (grpc/grpc-go#962).

Two experimental things added are: 1 the transport tap (grpc/grpc-go#968), 2. FailOnNonTempDialError (grpc/grpc-go#985), although that shouldn't bite us since you need to configure them separately.

So it looks like we should be kind of good.

@soltysh
Copy link
Member Author

soltysh commented Jan 11, 2018

The test error seems to be a known flake #11114. But
/retest

@mfojtik
Copy link
Member

mfojtik commented Jan 12, 2018

@smarterclayton @soltysh the diff looks scary, +4,355 −144 ? I still think that we should go with one cherry-pick to fix the issue and stay closer to upstream if possible.

@soltysh
Copy link
Member Author

soltysh commented Jan 12, 2018

The diff contains a quite a lot of .md files modifications. See one of my previous comments about the scope of the changes here.

Flake #15418.
/retest

@soltysh
Copy link
Member Author

soltysh commented Jan 12, 2018

/test all

@soltysh
Copy link
Member Author

soltysh commented Jan 15, 2018

Multiple time-outs during test.

/test all

@0xmichalis
Copy link
Contributor

/retest

@soltysh
Copy link
Member Author

soltysh commented Jan 23, 2018

Closing in favor of #17735.

@soltysh soltysh closed this Jan 23, 2018
@soltysh soltysh deleted the grpc_bump branch January 23, 2018 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. vendor-update Touching vendor dir or related files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants