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

Request only the Eio capabilities from env that are actually needed #220

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vog
Copy link

@vog vog commented Feb 11, 2025

This implements the recommendation "Passing env" from the Eio documentation:

Among others, this makes Piaf easier to use in applications where only the actually required capabilities are passed around, avoiding the need to define dummy capabilities just to fulfill Piaf's interface.

To provide a specific example, it helps to reduce code like this:

let http_request ~sw ~clock ~net ... =
  let env = object
    method clock = clock
    method net = net
    method stdin = assert false
    method stdout = assert false
    method stderr = assert false
    method domain_mgr = assert false
    method process_mgr = assert false
    method mono_clock = assert false
    method fs = assert false
    method cwd = assert false
    method secure_random = assert false
    method debug = assert false
    method backend_id = assert false
  end in
  let client = Piaf.Client.create env ~sw ... in
  ...

to that:

let http_request ~sw ~clock ~net ... =
  let env = object method clock = clock method net = net end in
  let client = Piaf.Client.create env ~sw ... in
  ...

Some remarks about the implementation:

  • The helper types required_env (client.ml) and clock (server.ml) simplify the type definition and casting required to put the Eio capabilities into a record.
  • The sub function order_v4v6 has been moved out of Connection.resolve_host to make the latter polymorphic in its env parameter. Otherwise, calling Connection.resolve_host would be more cumbersome, as each call would require an explicit downcast to required_env (or a copy of that type in Connection).
  • The definition of resolve_host could be simplified to resolve_host env ~config ~port hostname =, but I kept the current style to avoid messing around with the indentation, which would make the PR harder to read.

This implements the recommendation "Passing env" from the Eio documentation:
https://github.com/ocaml-multicore/eio#passing-env
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.

1 participant