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 revived #124

Closed
wants to merge 111 commits into from
Closed

Cohttp revived #124

wants to merge 111 commits into from

Conversation

vasilisp
Copy link
Contributor

@vasilisp vasilisp commented Jun 14, 2017

This is a new effort to layer Ocsigenserver over cohttp. It started with the #63 code (thanks @dinosaure, @Drup, @rgrinberg). The high-level difference is that all extensions operate directly on cohttp-friendly requests (Ocsigen_request.t) and responses (Ocsigen_response.t), as opposed to translating cohttp requests to Ocsigen requests, performing Ocsigen stuff in the middle, and translating the final Ocsigen response to a cohttp response.

The branch brings some other improvements not entirely related to cohttp. Notably:

  • There is a new way to configure the server without an XML file (Ocsigen_server.Site).
  • The Ocamlnet dependency is gone. We use the pcre bindings instead of Netstring_pcre.
  • The TyXML dependency is gone. We use xml-light in place of the Simplexmlparser in TyXML.
  • Transition to Lwt 3.0.

There are corresponding cohttp branches for Eliom and Ocsigen Start. The Eliom branch in particular needs thorough review.

There are a few missing pieces.

  • Not all extensions have been ported to the new APIs.

    • Cgimod
    • Comet (in my view it has been superseded by Eliom_comet).
  • Some extensions cannot yet be used via Ocsigen_server.Site. Meaningfully:

    • Revproxy
    • Rewritemod
    • Ocsipersist (work preferably on the separate repo)
    • Eliom (partially works but not all configuration is available through Ocsigen_server.Site).
  • Revproxy uses the Cohttp client, and we doubt it works the way it used to. See Reverse proxy (cohttp version): reuse connections #128.

  • More thorough testing.

  • Documentation, especially for the Ocsigen_server.Site API and related workflow.

  • Do we need Ocsigen_multipart, or is multipart-form-data good for our purposes? I have some outdated code for using the latter.

For now, you can open other PRs against the cohttp branch (NOT against master).

Feel free to open issues where discussion is needed, for the above points or for anything else.

vasilisp added 30 commits June 14, 2017 22:28
We need to correctly handle paths in Ocsigen_cohttp_server.Request for
this to work.
- store sub_path
- update function
- Abstract t type
- Replaces old ri_of_url
- Untested
Do not translate back and forth.

Do we really need this module? It doesn't do much on top of
Cohttp_lwt_unix.Client.
- Ocsigen_request
- Ocsigen_response
- Ocsigen_cohttp for what remains
We need Ocsigen_cookies on the client, but we don't want to link-in
Ocsigen_lib.
Faster, and perform much less memory allocations.
Fixes #117.
@Drup
Copy link
Member

Drup commented Feb 6, 2018

By pure curiosity, do you intend to switch to httpaf ?

@vasilisp
Copy link
Contributor Author

vasilisp commented Feb 6, 2018

@Drup httpaf is in our radar, but it is not 100% ready yet. Switching to it at this time would require a very significant investment of time.

@Drup
Copy link
Member

Drup commented Feb 6, 2018

@Drup httpaf is in our radar, but it is not 100% ready yet. Switching to it at this time would require a very significant investment of time.

Fair enough. I would think moving from cohttp to httpaf would be significantly easier than what you already did, but it's indeed a bit early.

@vasilisp vasilisp force-pushed the cohttp branch 7 times, most recently from 72382c8 to 2994ad6 Compare February 13, 2018 10:49
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