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

Appears not to support streaming #1

Open
aantron opened this issue Jul 5, 2021 · 2 comments
Open

Appears not to support streaming #1

aantron opened this issue Jul 5, 2021 · 2 comments

Comments

@aantron
Copy link

aantron commented Jul 5, 2021

e.g.:

let%lwt body = Dream.body response in
Lwt.return @@ with_encoded_body ~algorithm body response

This appears to read the entire body into memory, and then compress it. I assume (perhaps wrongly) that the underlying compression algorithms support streaming, and it seems like streaming would be more beneficial for precisely the kind of responses that you wouldn't want to load into memory an extra time (large ones).

@tmattio
Copy link
Owner

tmattio commented Jul 5, 2021

assume (perhaps wrongly) that the underlying compression algorithms support streaming

It does! In fact, the compression function (which is almost identical to decompress's documentation example) creates a stream from the given string, so refactoring this to work on stream responses should be trivial 🤞

I'll get to it as soon as I have some spare time

@aantron
Copy link
Author

aantron commented Jul 5, 2021

I'll get to it as soon as I have some spare time

No rush! And I think you will likely need some improvements in Dream's streaming APIs to get it done, so do bug me :)

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

2 participants