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

Serve site from fs #70

Merged
merged 3 commits into from
Aug 31, 2021
Merged

Serve site from fs #70

merged 3 commits into from
Aug 31, 2021

Conversation

tmattio
Copy link
Collaborator

@tmattio tmattio commented Jul 30, 2021

Updates the v3.ocaml.org static file serving to read the files from the filesystem instead of crunching them at compile time.

The path of the site dir can be set at runtime time with the OCAMLORG_SITE_DIR env variable.

cc @avsm

Lwt_io.(with_file ~mode:Input file) (fun channel ->
let open Lwt.Syntax in
let* content = Lwt_io.read channel in
Dream.respond ~headers content))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't Dream.from_filesystem better than constructing a manual response here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a custom handler because the requested path is not the path of the file we want to serve (i.e. we need to add an index.html if it's not there)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(The IO code is taken from Dream.from_filesystem)

@patricoferris
Copy link
Contributor

This LGTM if it is ready to go (I'm guessing this is the last thing blocking having continuous deployment via ocurrent deployer?)

@tmattio
Copy link
Collaborator Author

tmattio commented Aug 31, 2021

Thanks for the review @patricoferris!

I think it is, @avsm needed this when working on the deployment. I'll merge now.

@tmattio tmattio merged commit 9d3de10 into main Aug 31, 2021
@tmattio tmattio deleted the serve-site-from-fs branch September 8, 2021 19:53
tmattio pushed a commit that referenced this pull request Sep 14, 2021
tmattio pushed a commit that referenced this pull request Sep 14, 2021
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

Successfully merging this pull request may close these issues.

3 participants