retrieve the port number used by a listener.

[benoitc]

add cowboy:get_port/1 function to retrieve the port number used by
a listener.

[ghost]

I like this change in principle but i disagree with the implementation. We would rather open the listener socket higher up in the supervisor tree and propagate it down to the acceptors supervisor than open it down in the acceptor supervisor and send information about it back up again with a function that we wouldn't otherwise need.

The listener server should also be started with the socket returned from `Transport:listen' over the port number. This way all information that it provides is available - if needed at a later point.

[benoitc]

the question is how to open up higher. The reason I did it in the listener was because you already do that for some other information you are retrieving. Also what do you mean by "The listener server should also be started with the socket returned from `Transport:listen' over the port number".

Do you plan to change that soon? How could I help.

[ghost]

@benoitc With "The listener server should also be started with the socket returned from Transport:listen over the port number" i meant that it in my opinion it would be better to add the listen socket as an argument to the cowboy_listener:start_link function. That way this type of information is always reachable via the listener server.

@essen your two cents?

[ghost]

So, once the acceptor pool has been factored out in to ranch, this should go into ranch imo. Should be easy enough to rewrite the patch for ranch with the changes we discussed. @benoitc are we putting this on you're table or mine? whichever you prefer.

[Vagabond]

I think the changes to the transport are somewhat redundant given that my sockname patch was merged, which gives access to the IP/Port pair. A specific function for getting a port doesn't seem necessary IMO.

[benoitc]

I was traveling a lot these days, is ranch supposed to be used in cowboy as a dependency. If yes, sure this change should be done in ranch.

For the get_port or not, this is a more a convenience than anything, ie returning the port. But I don't see why ranch or cowboy couldn't offer this convenience to the user.

[ghost]

@benoitc I don't see why not either. The functions exported from the cowboy module aren't expected to map 1:1 with functions in the transport modules. We can reuse the sockname functions to implement a cowboy|ranch:get_listener_port/1 function.

[essen]

You're confusing things. @Vagabond was talking about the cowboy_tcp|ssl_transport get_port function, NOT the code to retrieve the listener's port. Cowboy current already has facilities to retrieve the accepted socket port. So that part of this PR became redundant.

[benoitc]

I'm not confusing things and was addressing this specific case.

Transport:sockname is returning a tuple {ok, {Adress, Port}} possibly . The point of Transport:get_port/1 is to return the port number. It's a convenience but sometimes really useful rather than letting your users do all the work. Which is also why you also have inets:port/1 I guess.

[essen]

Just to be sure we're talking about the same thing, you want:

Port = cowboy_tcp_transport:get_port(Socket)

Instead of:

{ok, {Addr, Port}} = cowboy_tcp_transport:sockname(Socket)

Is that right?

[benoitc]

One part of my patch is about that yes. The other is how to get the socket used by a listener.

[essen]

This will be merged into Ranch. Issue here: ninenines/ranch#4

Thanks!