Generalize Client
#72
Conversation
The type of `run` is ```haskell run :: ClientConfig -> Config -> Client a -> IO a ``` which looks reasonable enough. However, `Client` is a synonym for ```haskell type Client a = (Request -> (Response -> IO a) -> IO a) -> IO a ``` The client is given a `sendRequest` function which it can use to send requests, as long as it provides a way to process the response. That too is reasonable, but the confusing part is the use of `a` in the type of `sendRequest`: why does this have to be related to the final result of the `Client`? It seems to suggest that perhaps all of these are collected in some way, but of course that cannot be the case since (the result would have to be `IO [a]` or something). Indeed, if we look at the code, there _is_ no relation; in this commit we therefore generalize `Client` to ```haskell type Client a = (forall b. Request -> (Response -> IO b) -> IO b) -> IO a ``` This makes more sense: we get a callback that we can call as often as we we like; whatever type we get back when we process the response is the result of that callback; and we then use this callback to produce the final result, whatever it is. This requires _no_ code changes in the library at all; just needs `RankNTypes` enabled in a few modules. **Backwards compatibility**: this is strictly a generalization of the existing API, so existing code will continue to work; it just gets a more precise type, and can do more. _However_, there is one limitation, which is why I had to modify one of the tests in the test suite: where before we could have a `[Client ()]`, that is no longer possible (as it would require impredicativity). I suspect that this will not really be an issue, but I do not know for sure.
test/HTTP2/ServerSpec.hs
Outdated
| clients = [client0,client1,client2,client3,client3',client3'',client4,client5] | ||
|
|
||
| client :: C.Client () | ||
| client sendRequest = mapConcurrently_ id $ [ |
There was a problem hiding this comment.
I think we can use foldr1 concurrently_ instead of mapConcurrently_ id $ .
There was a problem hiding this comment.
Sure, that would be equivalent.
|
Pushed a commit that uses |
|
Rebased and merged. Thank you for your contribution! |
This was a remnant of kazu-yamamoto#72 (CI did not run, I guess).
|
@kazu-yamamoto , how does your release process work? I noticed that 4.1.4 includes #78 so I figured I'd give it a go instead of my local branch (I was going to try |
|
|
Ok, perfect, makes total sense! Re other PR, I'm inching closer towards that; lot of stuff needed to happen on my side before I could focus on http2 again but nearly there. |
This was a remnant of kazu-yamamoto#72 (CI did not run, I guess).
The type of
runiswhich looks reasonable enough. However,
Clientis a synonym forThe client is given a
sendRequestfunction which it can use to send requests, as long as it provides a way to process the response. That too is reasonable, but the confusing part is the use ofain the type ofsendRequest: why does this have to be related to the final result of theClient? It seems to suggest that perhaps all of these are collected in some way, but of course that cannot be the case since (the result would have to beIO [a]or something).Indeed, if we look at the code, there is no relation; in this commit we therefore generalize
ClienttoThis makes more sense: we get a callback that we can call as often as we we like; whatever type we get back when we process the response is the result of that callback; and we then use this callback to produce the final result, whatever it is.
This requires no code changes in the library at all; just needs
RankNTypesenabled in a few modules.Backwards compatibility: this is strictly a generalization of the existing API, so existing code will continue to work; it just gets a more precise type, and can do more. However, there is one limitation, which is why I had to modify one of the tests in the test suite: where before we could have a
[Client ()], that is no longer possible (as it would require impredicativity). I suspect that this will not really be an issue, but I do not know for sure.