Skip to content

[17.09] Increase gRPC request timeout to 20 seconds when sending snapshots#2404

Merged
nishanttotla merged 2 commits into
moby:bump_v17.09from
nishanttotla:increase-grpc-timeout-17.09
Nov 8, 2017
Merged

[17.09] Increase gRPC request timeout to 20 seconds when sending snapshots#2404
nishanttotla merged 2 commits into
moby:bump_v17.09from
nishanttotla:increase-grpc-timeout-17.09

Conversation

@nishanttotla
Copy link
Copy Markdown
Contributor

@nishanttotla nishanttotla commented Oct 11, 2017

Cherry-pick #2391

git cherry-pick -s -x e3e2821fe3eae707915b78215526da078d2d75a7

Cherry-pick was clean.

@nishanttotla
Copy link
Copy Markdown
Contributor Author

nishanttotla commented Oct 11, 2017

cc @andrewhsu @thaJeztah @anshulpundir

@nishanttotla nishanttotla force-pushed the increase-grpc-timeout-17.09 branch from 32c6678 to 06497c4 Compare November 7, 2017 20:33
@nishanttotla
Increase gRPC request timeout to 20 seconds when sending snapshots
b6e7720 
Signed-off-by: Nishant Totla <nishanttotla@gmail.com>
(cherry picked from commit e3e2821)
Signed-off-by: Nishant Totla <nishanttotla@gmail.com>
@nishanttotla nishanttotla force-pushed the increase-grpc-timeout-17.09 branch from 06497c4 to b6e7720 Compare November 7, 2017 20:33
@nishanttotla
Copy link
Copy Markdown
Contributor Author

nishanttotla commented Nov 7, 2017

@abhi I remember your PR had some vendoring issues like this one is having. Can you tell me what the fix was?

@nishanttotla
Rerunning vndr after version update
52342e2 
Signed-off-by: Nishant Totla <nishanttotla@gmail.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 7, 2017 •
edited
Loading

Codecov Report

Merging #2404 into bump_v17.09 will increase coverage by 0.05%.
The diff coverage is 100%.

@@               Coverage Diff               @@
##           bump_v17.09    #2404      +/-   ##
===============================================
+ Coverage        60.22%   60.28%   +0.05%     
===============================================
  Files              128      128              
  Lines            26154    26167      +13     
===============================================
+ Hits             15751    15774      +23     
+ Misses            9025     9007      -18     
- Partials          1378     1386       +8

@nishanttotla
Copy link
Copy Markdown
Contributor Author

nishanttotla commented Nov 7, 2017

Okay doing a vndr update and rerunning vndr seems to have fixed the CI.

Ping @anshulpundir @dperny

anshulpundir
anshulpundir reviewed Nov 7, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@anshulpundir anshulpundir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some readme files updated. Is this expected ? @nishanttotla

@nishanttotla
Copy link
Copy Markdown
Contributor Author

nishanttotla commented Nov 7, 2017

@anshulpundir it's because I reran vndr after updating the its version.

@nishanttotla
Copy link
Copy Markdown
Contributor Author

nishanttotla commented Nov 7, 2017

@andrewhsu can you confirm that this is okay to do?

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Nov 7, 2017

Yes; recent versions of vndr include readme's, because they often state licensing information

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Nov 7, 2017

Perhaps worth having something similar as in the moby/moby repo, where we pin to a specific version of vndr to make sure the vendoring is always done with the same version; https://github.com/moby/moby/blob/842bbeb63d82a55ee6ce537f860e9b37c146481c/hack/dockerfile/install-binaries.sh#L156

@dperny
Copy link
Copy Markdown
Collaborator

dperny commented Nov 8, 2017

:shipit:

@nishanttotla nishanttotla merged commit 1841adb into moby:bump_v17.09 Nov 8, 2017
@nishanttotla nishanttotla deleted the increase-grpc-timeout-17.09 branch November 8, 2017 19:50
@jose-bigio jose-bigio mentioned this pull request Nov 18, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@anshulpundir anshulpundir anshulpundir approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@nishanttotla @thaJeztah @dperny @anshulpundir