
On Sat, May 22, 2010 at 11:15 PM, Gregory Collins
Michael Snoyman
writes: Congratulations on the release. I was interested in seeing how this would work as a WAI handler, and came across some questions:
* I noticed that the Method datatype is restricted to a set of specific methods. Seeing as the list of methods can be expanded[1], why was this chosen?
The answer is "no particular reason" -- nobody really uses this. I think you're right that this makes us technically out of spec, if it becomes an issue for anyone we'll add a custom constructor later.
* The CIByteString datatype provides no way of accessing directly the lower-case version of the bytestring, or of setting it. Seeing as WAI already lower-cases the headers (following your suggestion btw) it would be more efficient to only do this once. Would you consider exposing the constructor?
Would you accept:
ciToLower :: CIByteString -> ByteString
instead? I prefer opaque datatypes in general. We didn't see a need for that use-case, the idea was that the string representation would be the same but we would do a case-insensitive compare.
Well, that solves half the problem: I also would like to be able to hand in a lower-case version when converting from the WAI ResponseHeader to a CIByteString.
* For simplicity at the moment, I decided to use the getRequestBody function, but it seems to be returning an empty result. Is there a known gotcha here?
If the POST body has content-type "application/x-www-form-urlencoded" we parse it for you and put the fields in the parameter mapping. If this isn't your case I'd appreciate a bug report. This is a place where we made a different design decision than you did -- a WAI application which expects to parse the POST body itself won't work. I'll think about adding a knob to make this behaviour optional.
I'd appreciate such a knob. Out of curiosity, do you also parse multi-part
data?
Overall, writing the WAI wrapper is pretty straight-forward. The main problem is that the WAI request body does not require an inversion of control approach, while the Snap version does; some usage of lazy I/O here could solve the problem, though that's obviously sub-optimal.
A Chan & a forkIO could work here also (we've discussed that one before I think.)
You're right; it doesn't actually require the lazy I/O bit, just the
forkIO. Not that I'm crazy about that solution either.
Also, it seems a little unclear whether the writeBS et al functions store the body in memory before returning the result, though the documentation implies it. Could you provide some clarifications?
The Snap monad carries a "Response" in its state, with "rspBody" being an output Enumerator. The "writeBS" function composes "enumBS foo" with the Enumerator from that state; so when you call writeBS you're really building up a *program* to send the response body out later. So yes, we hang onto that data until the request is served.
That's what it looked like to me; maybe you could update the docs to make
that clear. I'd hate for people to accidentally kill their performance. If you want to stream in O(1) space you need to provide an Enumerator to
do so; the Enumerator has access to the IO monad though. We provide writeBS/writeLBS for those situations in which it's convenient/appropriate to build up the entire response in memory.
I figured I'd end up writing that, but I just wanted to test things out the simple way first via writeLBS. I'll have to wait in any event for the ability to bypass automatic request body parsing.
Michael