Skip to content
This repository has been archived by the owner on Nov 7, 2019. It is now read-only.

implement TryFrom<std::net::UdpSocket/TcpListener> for UdpSocket/TcpListener #99

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

vavrusa
Copy link

@vavrusa vavrusa commented Jul 25, 2019

This allows setting various socket options (e.g. SO_REUSEPORT) before
binding to the socket, and also socket passing from the service supervisor (e.g. systemd).

vavrusa added 2 commits July 25, 2019 15:15
TryFrom is implemented for std::net::{UdpSocket, TcpListener} respectively.

This allows setting various socket options (e.g. SO_REUSEPORT) before
binding to the socket, and also socket passing from the service supervisor (e.g. systemd).
@vavrusa vavrusa changed the title udp: implement TryFrom<std::net::UdpSocket> for UdpSocket implement TryFrom<std::net::UdpSocket/TcpListener> for UdpSocket/TcpListener Jul 25, 2019
@yoshuawuyts
Copy link
Collaborator

Hey thanks so much for this patch!

For work we encountered a similar situation recently (around mio also), and something that came out of it is that if a std component can't be changed to a mio variant, something terrible has gone wrong, and it's probably safe to abort the process.

Which is to say: I think it'd be entirely reasonable to use From rather than TryFrom for these conversions.

@kpp
Copy link
Contributor

kpp commented Jul 29, 2019

Which is to say: I think it'd be entirely reasonable to use From rather than TryFrom for these conversions.

Let the user fail as they want =)

@vavrusa
Copy link
Author

vavrusa commented Jul 29, 2019

It will probably be okay, but I didn't feel confident burying .unwrap() in a fundamental library for just a slightly easier use (this is also not something that will be used that often I guess). That being said, I'm happy to change it if your consensus is to go with From trait instead.

@vavrusa
Copy link
Author

vavrusa commented Aug 2, 2019

Just to clarify - the "👍" means "it's ok to keep it as it is", "please change it to From", or "undecided" ?

@kpp
Copy link
Contributor

kpp commented Aug 2, 2019

I am against .unwrap() in a library code

@yoshuawuyts
Copy link
Collaborator

@vavrusa yeah, I'd prefer if it was From rather than TryFrom. If this fails then something terrible has gone wrong on the mio side, and it's probably best to abort.

@kpp Sorry, but that feels like an arbitrary restriction. If we were to follow that then romio in its current form wouldn't even be able to exist.

@kpp
Copy link
Contributor

kpp commented Aug 2, 2019

Sorry, but that feels like an arbitrary restriction.

It's not. You can add both TryFrom and From, but implement From as try_from().unwrap(). Having only From implemented with .unwrap() is a restriction.

@yoshuawuyts yoshuawuyts force-pushed the master branch 2 times, most recently from a989815 to 617f76d Compare September 23, 2019 10:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants