ManagerSettings.managerConnectionTimeout

[nikomi]

Would you consider adding a separate ManagerSettings.managerConnectionTimeout in addition to managerResponseTimeout so the connection timeout can be different from the response timeout?

A bit of background:

We built a Manager extension with circuit breaker functionality, using Request.getConnectionWrapper to enforce a connection timeout different from the configurable response timeout. With the API change from http-client 0.4.x → 0.5.x getConnectionWrapper was dropped, therefore we can no longer support this feature in this way. Instead of bringing back Request.getConnectionWrapper we believe adding ManagerSettings.managerConnectionTimeout would probably benefit more people.

[snoyberg]

I'm OK with such a change if it doesn't significantly complicate the code base. I'd imagine it should be possible to piggy-back on the managerResponseTimeout code already present. What is your proposed signature for managerConnectionTimeout? How would it interact with managerResponseTimeout values?

…

On Tue, Aug 29, 2017 at 4:01 PM, nikomi ***@***.***> wrote: Would you consider adding a separate ManagerSettings. managerConnectionTimeout in addition to managerResponseTimeout so the connection timeout can be different from the response timeout? A bit of background: We built a Manager extension with circuit breaker functionality, using Request.getConnectionWrapper to enforce a connection timeout different from the configurable response timeout. With the API change from http-client 0.4.x → 0.5.x getConnectionWrapper was dropped, therefore we can no longer support this feature in this way. Instead of bringing back Request.getConnectionWrapper we believe adding ManagerSettings. managerConnectionTimeout would probably benefit more people. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#297>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADBB_o2CRN3m4b9hSnhpWRf302Zlyo8ks5sdAu9gaJpZM4PF5MS> .

[nikomi]

Thanks for considering the change!

I think the signature could be the same as managerResponseTimeout - either reusing the ResponseTimeout type or duplicating it into an equivalent ConnectionTimeout type. The latter would allow forcing a connection timeout to be set by not offering a 'no timeout' option.

managerConnectionTimeout should be used whenever a new connection is established, and managerRequestTimeout whenever an actual request is sent, waiting for the response.

[snoyberg]

I think there may be some issues with getting these two mechanisms to coexist cleanly, without instead making one supersede the other, but I could be wrong. If you want to take a stab at a PR, I'll be happy to review.

[nikomi]

I'm afraid I have no clue how http-client works internally, and I believe the solution should be something that fits your design so you are fully confident in it.

I'm not sure I do understand your concerns, I thought that the socket connect and socket send steps would be rather separate and could there use separate timeouts. Maybe I confused matters by discussing data types - the basic hope is just separately configurable timeouts for new connections and requests.

[snoyberg]

My point is that, currently, the response timeout covers both the connection and first successful recv period. Adding a separate timeout that somewhat overlaps in behavior can be confusing if not implemented and documented correctly.

I'm not going to have a chance to attack this one myself any time soon, so if someone else wants to take a stab at it, feel free to speak up here.

[nikomi]

Ah, I see - yes, this would definitely need proper documentation.

In terms of PR - I'll leave this open for a while and see what I can do.

[nikomi]

I'm sorry I did not find the time to work on this... but before closing this I have question:

Since we are still using http-client 0.4 - which does not compile with GHC 8.4 and we therefore need to upgrade in the forseeable future - we would like to know if you already have plans (and possibly a time frame) for the next API change, probably called http-client 0.6?

[snoyberg]

I have no plans currently.

[nikomi]

Thanks for the info, I'll close this one as it does not look like I'll be getting it done soon...