Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Have a streaming request body for servant-client #886

Closed
nmattia opened this issue Jan 16, 2018 · 5 comments
Closed

Have a streaming request body for servant-client #886

nmattia opened this issue Jan 16, 2018 · 5 comments

Comments

@nmattia
Copy link

nmattia commented Jan 16, 2018

Currently, the request body may only be a lazy bytestring. This is a problem when large files must be included in the body, as the file must either be loaded fully in memory, or one has to use lazy I/O. It would be nice to have something similar to http-client's request body. I understand that it's not doable for servant-client-core to depend on http-client; however there may be a way to extract some types from http-client, which would also make the multipart implementation ( #884 ) simpler.

CC @jkarni @alpmestan

@alpmestan
Copy link
Contributor

@nmattia Something like this (and we have a server interpretation for this as well) ? Note that this points to the 'latest' branch of the docs, which reflects the state of master, this streaming stuffs has not been released yet, it will be part of the next one.

One little note: the Stream stuffs are completely separate from Get, Post etc currently, which is not very elegant. We have ideas for unifying everything in #841.

Also, regarding a client interpretation for multipart, someone's interesting in working on this and I described a possible way to tackle this in here.

@nmattia
Copy link
Author

nmattia commented Jan 17, 2018

@nmattia Something like this (and we have a server interpretation for this as well) ? Note that this points to the 'latest' branch of the docs, which reflects the state of master, this streaming stuffs has not been released yet, it will be part of the next one.

Not sure, I didn't look at it in much detail but it seems it be related to server responses, whereas I'm looking for a streaming Request/RequestBody. Moreover I don't believe the types would change much, a new constructor would be added to RequestBody.

Also, regarding a client interpretation for multipart, someone's interesting in working on this and I described a possible way to tackle this in here.

Right, I posted my implementation in there. As mentioned in my first message here, the unfortunate trade-off is loading the file in memory vs. lazy I/O (I went with the former here).

@alpmestan
Copy link
Contributor

Oh right, we don't have a streaming request body thing there, I thought we did for a second sorry.

Regarding your implementation, that's a really great start, but the choice between in-memory vs lazy I/O is not one I'm really willing to make.

But indeed the proper solution is to just add a constructor to represent "files to be sent" or maybe something a little more general, in servant-client-core, so that each backend can then do the smartest thing when it comes to actually performing a request that involves sending the contents of a bunch of files over to the server. We must simply keep in mind that we need the ability to send both files and some "direct" input in the same request body, so the new constructor should not just be about files. And this is what we should aim for in my opinion, as described in the other issue.

@jkarni
Copy link
Member

jkarni commented Jan 17, 2018

Anyone understand the point of RequestBodyIO?

@phadej
Copy link
Contributor

phadej commented Jan 17, 2018

Related: #656, time have past though, so doing that exercise again is not straight forward.

@jkarni iRequestBodyIO is what the docs say:

`> Allows creation of a RequestBody inside the IO monad, which is useful for making easier APIs (like setRequestBodyFile

interesting part is RequestBodyStream

RequestBodyStream Int64 ( (IO ByteString -> IO ()) -> IO ()) -- GivesPopper ()

When you look at that long enough, you see that's it's used as

case body of
    RequestBodyStream _len givespopper = givespopper writer where
        writer :: IO ByteString -> IO ()
        writer popper = do
            bs <- popper
            if BS.null bs
            then weAreDone
            else writeBs bs >> writer popper

... and the ContT wrapping is probably to handle resources (i.e. not just giving popper directly).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants