Conversation
httpev.ml
Outdated
| match addr, Systemd.Daemon.listen_fds_lwt () with | ||
| | None, [] -> | ||
| log#error "bind not provided and no systemd socket available"; | ||
| exit 1 |
There was a problem hiding this comment.
can't use exit here, one might want to recover from the error or try again
httpev.ml
Outdated
| let server_lwt config answer = | ||
| Lwt_main.run @@ setup_lwt config answer | ||
|
|
||
| let http_bind addr http_config = |
There was a problem hiding this comment.
I think this function should be named setup_<something> because it's what it does. E.g. setup_bind_lwt.
level-a
force-pushed
the
httpev/http_bind
branch
from
be187d3 to
0fd77bb
Compare
Khady
left a comment
Khady
left a comment
There was a problem hiding this comment.
looks ok. I am not sure if it's useful or not to return the signature. But I leave that to you.
|
can this be merged? |
setup http_server with http_config binded to systemd socket (by default) or to provided addr (host:port)
level-a
force-pushed
the
httpev/http_bind
branch
from
0fd77bb to
4703287
Compare
Khady
left a comment
Khady
left a comment
There was a problem hiding this comment.
looks good. I wonder if there should be a verbose argument and log only when it is true
| let server_lwt config answer = | ||
| Lwt_main.run @@ setup_lwt config answer | ||
|
|
||
| let setup_bind_lwt addr config answer = |
There was a problem hiding this comment.
there is config.connection which this api totally ignores which is not cool. Instead we should break api and make config.connection a variant itself and make all setup_* functions honor that variant and provide convenience functions to set that variant
4 participants
setup http_server with http_config binded to systemd socket (by default) or to provided addr (host:port)