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

allow keepalive option in ranch_tcp and ranch_ssl transports #109

Closed
wants to merge 1 commit into from

Conversation

gbour
Copy link

@gbour gbour commented Apr 21, 2015

related to #108

@essen
Copy link
Member

essen commented May 12, 2015

Alphabetical order on the list of options (at all 4 places you changed), thanks!

@gbour
Copy link
Author

gbour commented May 15, 2015

done :)

@essen
Copy link
Member

essen commented May 15, 2015

Cool. Make that a single commit (git rebase -i HEAD^^^) and squash the right commit and it'll be merged as soon as Monday.

@voltone
Copy link

voltone commented May 20, 2015

@gbour Any chance you can add 'ipv6_v6only' as well? Without it I cannot start listeners on the same port for IPv4 and IPv6. Assuming @essen doesn't have any objections...? Thanks!

@essen
Copy link
Member

essen commented May 20, 2015

I don't have objections but you should probably send a PR directly. :-)

@voltone
Copy link

voltone commented May 21, 2015

I was about to when I came across this issue, and I figured my PR would have a merge conflict with this one.

Would you consider a PR that changes socket option filtering to use a black-list rather than a white-list? I've ran into this issue a couple of times: first with the 'packet' option and then with 'keep-alive' (both of which shouldn't concern the listener socket, and I now set from my handler's init/4), and now with 'ipv6_v6only' (which cannot be set/changed after listen/2). I understand that some setting would interfere with the operation of the listener/acceptor logic in ranch, but it should be possible to filter out those. Maybe for v2?

@gbour
Copy link
Author

gbour commented May 21, 2015

@voltone I agree with @essen, you should fill a distinct PR.
@essen I merged commits, is it ok ?

@essen
Copy link
Member

essen commented May 21, 2015

Yes, sorry been distracted with erlang.mk, it will be merged as soon as I'm done.

@essen
Copy link
Member

essen commented Aug 18, 2015

Done in 0d5d855 along with many more options, and including documentation.

@voltone I did not add ipv6_v6only yet, because I am a little confused by its description. What's the default value for example? True? I am assuming you want to set it to false?

@essen essen closed this Aug 18, 2015
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.

3 participants