search

rgrinberg / opium

Sinatra like web toolkit for OCaml
MIT License
755 stars 67 forks source link

Provide of_file API to stream files as response #244

Closed joseferben closed 3 years ago

joseferben commented 3 years ago

Hey all,

I just finished updating Sihl to use Opium 0.19.0. This is the only API that I miss so far. I tried to find a sensible location for the read function, just let me know if you have any suggestions.

joseferben commented 3 years ago

Looks good, but the error handling in Body.of_file should be changed to not returns HTTP status, or removed completely.

Done.

I'm also wondering how this plays with Handler.serve, which is pretty much the same thing, except that it takes a custom read function:

  • Should of_file be exposed in Handler?
  • Should Handler.serve be exposed in Response, perhaps with an optional argument in of_file?

Yes they indeed look similar. The goal of Handler is to provide a set of ready-made handlers that users can plug in? I could allow for a optional read function in Response.of_file and use it to implement the handler.

Also, maybe out of scope here, but it'd be nice to have the counterpart Request.to_file to save the content of the request as a file on the filesystem. (There's a file upload example that does that if I recall correctly).

Wold it make sense to have both ways on both Request and Response?