allow IPAddr in serve #145
Conversation
|
Do you see other similar needs at this point? Will probably merge and release tomorrow morning if not, after another look. |
|
Thanks, a release would be great. As far as I can see, this was the only thing that needed fixing on the WebSockets side. |
| @@ -347,7 +350,7 @@ After a suspected connection task failure: | |||
| """ | |||
| function serve(serverws::ServerWS, host, port, verbose) | |||
| # An internal reference used for closing. | |||
| tcpserver = Ref{Union{IOServer, Nothing}}(Sockets.listen(InetAddr(parse(IPAddr, host), port))) | |||
There was a problem hiding this comment.
It would be nice if
IPv4(host::IPv4)
existed in Sockets, just like
Int(i::Int)
exists. But it doesn't, just like ServerWS(s::ServerWS) doesn't exist.
src/HTTP.jl
Outdated
| @@ -322,6 +322,9 @@ function ServerWS(h::Function, w::Function; kwargs...) | |||
| ServerWS(RequestHandlerFunction(h), WSHandlerFunction(w); kwargs...) | |||
| end | |||
|
|
|||
| to_IPAddr(ip::IPAddr) = ip | |||
There was a problem hiding this comment.
I suppose this is good style, considering. Alternatively, we could add
IPv4(i::IPv4) = ip
but risk that Sockets add a method like that later, and we'd end up with warnings. What do you think?
We need a test for this. But there's also other new methods that are not posted. Will have a look at current test coverage and probably submit a PR to get back up to ~100% coverage.
There was a problem hiding this comment.
I've followed HTTP convention (which is in line with Julia Base) to lowercase IPAddr in a function name (so now it's to_ipaddr) and have added tests for it. Adding a IPv4(::IPv4) method here would be type piracy: something like that should be added in the Sockets stdlib directly.
|
I've added tests and addressed the feedback, I think this should be good to merge. |
| @info "ServerWS: Open, http response, close. Repeat three times. Takes a while." | ||
| for i = 1:3 | ||
| let | ||
| server = startserver() | ||
| ip = parse(IPAddr, SURL) | ||
| server = startserver(url=ip) |
There was a problem hiding this comment.
To make sure that we test serving from a IPAddr at least once in the test suite.
|
Absolutely magnificent! One less for issue #147. |
Mux uses this code and passes the IP address directly (say
localhost = ip"0.0.0.0"::IPAddr). In this wayservesupports both passing the string and passing theIPAddrobject.