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

Make request I/O pure and expose the fd #82

Open
wants to merge 2 commits into
base: 0.4.0-gamma
Choose a base branch
from
Open

Make request I/O pure and expose the fd #82

wants to merge 2 commits into from

Conversation

aantron
Copy link

@aantron aantron commented Jun 12, 2017

  • Adjusts dispatch and helpers so that they do not write to the request fd. Instead, they return lists of writing operations, to be performed by the same outer code that reads from the fd, and calls dispatch.
  • Exposes the fd in the API.

| (Destroy k) as exn -> IO.fail exn
| exn ->
log_error ("Unknown exception caught: "^(Printexc.to_string exn));
IO.(Out.write_error log_error req Errno.EIO
>>= fun () -> fail exn)
Copy link
Author

Choose a reason for hiding this comment

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

FYI this line is the only reason why requests return a response list, instead of just a single response.

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes, this mechanism is rather messy... I think it can probably be simplified but continue your work as-is for now.

Copy link
Author

Choose a reason for hiding this comment

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

One option for faithfully encoding this without a list is to add another variand, something like `Error_and_raise _.

Signed-off-by: Anton Bachin <antonbachin@yahoo.com>
Signed-off-by: Anton Bachin <antonbachin@yahoo.com>
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.

None yet

2 participants