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

cohttp-eio: expose the underlying flow additionally to Buf_write when sending the response body #970

Open
TheLortex opened this issue Feb 15, 2023 · 3 comments

Comments

@TheLortex
Copy link
Member

Currently the body type is:

  type t =
    | Fixed of string
    | Chunked of chunk_writer
    | Custom of (Eio.Buf_write.t -> unit)
    | Empty

I believe it would be nice to have an additional body type that enables to write directly into the connection flow.

| Flow of (Eio.Flow.sink -> unit)

This way, one can use Eio.Flow.copy to send a file in a streaming fashion. I believe that using Buf_write would require writing the full content in the buffer, right ?

@bikallem
Copy link
Contributor

bikallem commented Feb 16, 2023

I believe that using Buf_write would require writing the full content in the buffer, right ?

Not sure but for your particular use-case can Eio.Buf_write.flush (https://github.com/ocaml-multicore/eio/blob/main/lib_eio/buf_write.mli#L247) help? i.e. IIUC flushing should ensure that you don't wait for the scheduled batched write. @talex5 can you confirm?

@talex5
Copy link
Contributor

talex5 commented Feb 17, 2023

@bikallem:

i.e. IIUC flushing should ensure that you don't wait

The link says:

[flush t] waits until all prior writes have been successfully completed.

@TheLortex:

I believe that using Buf_write would require writing the full content in the buffer, right?

What you can do is use schedule_cstruct to avoid a copy.

I'm not sure providing the raw sink will work, because we'll still typically need to do chunking. What are you planning to use it for?

@TheLortex
Copy link
Member Author

What are you planning to use it for?

I wanted to fix this TODO in eeww: https://github.com/avsm/eeww/blob/main/src/main.ml#L78

   |`Regular_file ->
      (* TODO stream body instead of loading *)
      let body = Eio.Path.load fname in
      Cohttpx.respond_file fname body

When serving files over HTTP, it's useful to be able to manipulate the flow so that a single copy is sufficient to stream the content of the file:

  let respond_file fname =
    ...
    let headers = Http.Header.of_list
      [ "content-type", mime_type;
        "content-length", string_of_int length;
      ] in
    let response =
      Http.Response.make ~version:`HTTP_1_1 ~status:`OK ~headers ()
    in
    response, Body.Flow (fun flow ->
      Eio.Path.with_open_in fname @@ fun file ->
      Eio.Flow.copy file flow)

without having to resort to the chunked encoding.

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

3 participants