Adding default Read/WriteTimeouts & configs for overriding #37
Conversation
| readTimeout = 10 * time.Second | ||
| // writeTimeout is used by the http server to set a maximum duration before | ||
| // timing out write of the response. The default timeout is 10 seconds. | ||
| writeTimeout = 10 * time.Second | ||
| // JSONContentType is the content type that will be used for JSONEndpoints. |
There was a problem hiding this comment.
Might as well just change the comment to jsonContentType while you're at it.
|
LGTM |
|
@oliver-nyt, I got that comment (33b39b8), thanks! |
|
🎉 🚙 |
| @@ -112,6 +112,8 @@ func (s *SimpleServer) Start() error { | |||
| srv := http.Server{ | |||
| Handler: RegisterAccessLogger(s.cfg, s), | |||
| MaxHeaderBytes: maxHeaderBytes, | |||
| ReadTimeout: readTimeout, | |||
| WriteTimeout: writeTimeout, | |||
There was a problem hiding this comment.
The values in this http.Server struct are duplicated here and server/rpc_server.go. It's not a big deal, but if they're always going to be synchronized, it might be worth extracting to some other place.
There was a problem hiding this comment.
Maybe even get rid of those global vars that are used to configure the servers.
There was a problem hiding this comment.
I agree, RPC needs some love and refactoring, but it's a bit out of the scope of this change. (Maybe we make an issue/enhancement?)
In the end, I imagine/hope the guts of the RPC server would get replaced by a bunch of go-kit transports and middleware, which is why I haven't coupled the two servers thus far.
Adding default Read/WriteTimeouts & configs for overriding
|
For future reference, some of the changes in this PR are to deal with: golang/go#12933 |
The existing Gizmo SimpleServer and RPCServer had no timeouts (and no options to set timeouts) for their HTTP Servers.
This PR adds a 10s default timeout for both and the ability to override them via
config.Server.