Change default connection time bound calculation from 5s to 5min.#107
Change default connection time bound calculation from 5s to 5min.#107creichert merged 1 commit intosnoyberg:masterfrom
Conversation
|
@creichert https://github.com/snoyberg/keter/blob/master/etc/keter-config.yaml#L36 It supposed to be milliseconds. Sorry, I should put description commentary next time. Excuse me so many issues by single PR. |
|
Hmm, what am I missing here? I still think milliseconds is not correct. If the intent is to configure in milliseconds then it needs to be multiplied by 1000 somewhere. The current default is The timeout function reverse-proxy is a lifted version of the timeout function in base, which is in microseconds: https://hackage.haskell.org/package/base-4.4.1.0/docs/System-Timeout.html#v:timeout |
|
Yep, the idea was to convert milliseconds from config to microseconds, I supposed that milliseconds is more suitable for connection bound rather than microseconds. |
|
That's fine with me, but the current code makes no conversion from milliseconds to microseconds for the actual timeout function so I wanted to clarify. |
|
To be clear, I am not going to merge the current patch. Instead, I'm going to multiply |
…e proxy. The connection-time-bound is configured by the user as milliseconds but eventually passed to the 'System.Timeout.Lifted.timeout' function which accepts microseconds. Added commentary at usage sites.
Change default connection time bound calculation from 5s to 5min.
|
I'm merging this now. I looked at several other servers and milliseconds is not out of the ordinary for configurations. @snoyberg I think this we need to cut a new release for this issue (the bug was reported to me on irc). |
|
Created a new pull for version bump and changelog update: #109 |
|
Cool! Apologize once again! |
@geraldus
@tolysz
I believe there was a small issue with the 5 minute calculation, I couldn't find anywhere that explicitly states that the value is in microseconds but after some discussion on IRC and further testing I believe this is correct.