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

Don't use wrappers for grpc metadata #2610

Merged
merged 1 commit into from
Apr 19, 2018
Merged

Conversation

crosbymichael
Copy link
Contributor

@crosbymichael crosbymichael commented Apr 17, 2018

This switches the current usage of NewContext and FromContext to use the
underlying methods in the metadata package. The wrappers will be removed in
future versions of grpc.

Signed-off-by: Michael Crosby crosbymichael@gmail.com

This switches the current usage of NewContext and FromContext to use the
underlying methods in the metadata package.  This will be removed in
future versions of grpc.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
@codecov
Copy link

codecov bot commented Apr 17, 2018

Codecov Report

Merging #2610 into master will decrease coverage by 0.16%.
The diff coverage is 37.5%.

@@            Coverage Diff             @@
##           master    #2610      +/-   ##
==========================================
- Coverage   61.74%   61.58%   -0.17%     
==========================================
  Files         134      134              
  Lines       21763    21763              
==========================================
- Hits        13438    13403      -35     
- Misses       6869     6914      +45     
+ Partials     1456     1446      -10

@thaJeztah
Copy link
Member

thaJeztah commented Apr 17, 2018

Had the same changes in #2452 (bd2f5d9) (looks like that needs a rebase); if we actually bump the versions, some tests needed updating; ee0bb7a

@crosbymichael
Copy link
Contributor Author

@thaJeztah i know. not bumping with this change for that reason

Copy link
Contributor

@cyli cyli left a comment

Choose a reason for hiding this comment

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

@dperny
Copy link
Collaborator

dperny commented Apr 19, 2018

Is there a functional difference between these two that we need to care about? If not, LGTM.

@cyli
Copy link
Contributor

cyli commented Apr 19, 2018

@dperny I believe the one we're using was deprecated and then finally removed. It just calls the new one.

@thaJeztah
Copy link
Member

@dperny @cyli correct; see my comment on the other pr: #2452 (comment)

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@anshulpundir anshulpundir merged commit 33d06bf into moby:master Apr 19, 2018
thaJeztah added a commit to thaJeztah/docker that referenced this pull request Apr 19, 2018
Updates swarmkit to 33d06bf, to bring in
moby/swarmkit#2610 (Don't use wrappers for grpc metadata)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
docker-jenkins pushed a commit to docker-archive/docker-ce that referenced this pull request Apr 26, 2018
Updates swarmkit to 33d06bf5189881b4d1e371b5571f4d3acf832816, to bring in
moby/swarmkit#2610 (Don't use wrappers for grpc metadata)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: b18f7033b2644c0246345bb5747a5436568b1a71
Component: engine
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants