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

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

Merged
merged 2 commits into from Nov 8, 2017

Conversation

Projects
None yet
4 participants
@nishanttotla
Contributor

nishanttotla commented Oct 11, 2017

Cherry-pick #2391

git cherry-pick -s -x e3e2821fe3eae707915b78215526da078d2d75a7

Cherry-pick was clean.

@nishanttotla

This comment has been minimized.

Show comment
Hide comment
@nishanttotla
Contributor

nishanttotla commented Oct 11, 2017

Increase gRPC request timeout to 20 seconds when sending snapshots
Signed-off-by: Nishant Totla <nishanttotla@gmail.com>
(cherry picked from commit e3e2821)
Signed-off-by: Nishant Totla <nishanttotla@gmail.com>
@nishanttotla

This comment has been minimized.

Show comment
Hide comment
@nishanttotla

nishanttotla Nov 7, 2017

Contributor

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

Contributor

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?

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

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Nov 7, 2017

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

codecov bot commented Nov 7, 2017

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

This comment has been minimized.

Show comment
Hide comment
@nishanttotla

nishanttotla Nov 7, 2017

Contributor

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

Ping @anshulpundir @dperny

Contributor

nishanttotla commented Nov 7, 2017

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

Ping @anshulpundir @dperny

@anshulpundir

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

@nishanttotla

This comment has been minimized.

Show comment
Hide comment
@nishanttotla

nishanttotla Nov 7, 2017

Contributor

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

Contributor

nishanttotla commented Nov 7, 2017

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

@nishanttotla

This comment has been minimized.

Show comment
Hide comment
@nishanttotla

nishanttotla Nov 7, 2017

Contributor

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

Contributor

nishanttotla commented Nov 7, 2017

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

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Nov 7, 2017

Member

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

Member

thaJeztah commented Nov 7, 2017

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

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Nov 7, 2017

Member

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

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

This comment has been minimized.

Show comment
Hide comment
@dperny
Member

dperny commented Nov 8, 2017

:shipit:

@nishanttotla nishanttotla merged commit 1841adb into docker:bump_v17.09 Nov 8, 2017

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
codecov/project 60.28% (target 0%)
Details
dco-signed All commits are signed

@nishanttotla nishanttotla deleted the nishanttotla:increase-grpc-timeout-17.09 branch Nov 8, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment