-
-
Notifications
You must be signed in to change notification settings - Fork 134
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
Add support for listening to unix domain sockets #219
Conversation
@@ -2193,6 +2193,7 @@ val catch : (error -> response promise) -> middleware | |||
val run : | |||
?interface:string -> | |||
?port:int -> | |||
?socket_path:string -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it would be a cleaner API to offer separate functions for serving on a port (which would likely be used more) vs on a "network address", which could correspond to the network
type defined elsewhere in this PR? Feels like we shouldn't accept multiple config parameters if some of them could get cancelled out. Possibly run_sock
and serve_sock
?
Doesn't look like interface
is needed if listening on a unix socket either; pretty sure is_localhost
would just need to be adapted to return true
if on a socket.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a socket_source
instead of interface
, port
or socket_path
can be passed. Inet
or Unix
being possible kinds of sources. Although that will make the run function slightly more cumbersome, but the implementation can just default to Inet(defaultInterface, defaultPort)
.
src/http/http.ml
Outdated
@@ -379,13 +379,15 @@ let built_in_middleware error_handler = | |||
Catch.catch (Error_handler.app error_handler); | |||
] | |||
|
|||
|
|||
type network = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not reuse Unix.sockaddr
?
Thank you! I'll add docs in a follow-on commit. |
Fix #218