ContextService type for SimpleServer #21
Conversation
| ctx = context.WithValue(ctx, "forward-for-ip", ip) | ||
| } | ||
|
|
||
| id, err := GenerateRequestID() |
There was a problem hiding this comment.
Other folks may start adding additional tracing via go-kit/kit/tracing or some other tooling so we might not want to have this in here by default.
Would you mind abstracting this chunk with the RequestID into a middleware func? Then you can just add it into the ContextMiddleware method of your ContextService implementation.
Also, for the context keys, would you mind creating a separate type for them? I'd like us to match the pattern covered in the Go context blog post. (example: https://blog.golang.org/context/userip/userip.go)
|
Great start, @gaplyk! After you make your changes, mind squashing your commits? http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html |
| } | ||
|
|
||
| var ( | ||
| cancel context.CancelFunc |
There was a problem hiding this comment.
Mind changing it to a one liner since we only have a single variable being declared?
| type ContextKey int | ||
|
|
||
| // UserIPKey is key to set/retrieve value from context | ||
| const UserIPKey contextKey = 0 |
There was a problem hiding this comment.
Mind tossing the consts in a const ( ) block? Just visually groups them together a bit more.
gaplyk
force-pushed
the
master
branch
2 times, most recently
from
40b5380
to
ffc30c2
Compare
| @@ -62,3 +63,25 @@ type RPCService interface { | |||
|
|
|||
| // JSONEndpoint is the JSONService equivalent to SimpleService's http.HandlerFunc. | |||
| type JSONEndpoint func(*http.Request) (int, interface{}, error) | |||
|
|
|||
| // ContextService is an interface defining a service that | |||
| // is made up of ContextHandler | |||
There was a problem hiding this comment.
Mind adding periods to these comments?
ContextService type for SimpleServer
|
Thanks, @gaplyk! 💥 |
New context Service which using "golang.org/x/net/context"