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

Use per request context as provided by net/http. (deprecates server wide context) #480

Merged
merged 2 commits into from
Mar 5, 2017

Conversation

basvanbeek
Copy link
Member

Since go 1.7 we have per request context available. This eliminates the need for server wide scoped context. Since this is also our now lowest supported version we can move away from the old implementation.

This PR removes using the server wide context inside the ServeHTTP handler in favor of the one provided by *http.Request.

Examples and tests have been updated to reflect the change too.

@yurishkuro
Copy link
Contributor

+1

I am not even sure there was a static context in the api, since context is a request-scoped concept.

@peterbourgon
Copy link
Member

LGTM! Merge at your leisure.

@basvanbeek basvanbeek merged commit bd066f4 into go-kit:master Mar 5, 2017
@basvanbeek
Copy link
Member Author

basvanbeek commented Mar 19, 2017

See: https://golang.org/pkg/context/

Package context defines the Context type, which carries deadlines, cancelation signals, and other request-scoped values across API boundaries and between processes.

With both HTTP and gRPC transports providing a per request generated Context server side we should really use those.

Is there anything you were doing with context values before passing them to NewServer?

@basvanbeek basvanbeek deleted the http-request-context branch December 24, 2017 00:12
jamesgist pushed a commit to jamesgist/kit that referenced this pull request Nov 1, 2024
Use per request context as provided by *http.Request
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.

4 participants