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

Use std::net::SocketAddr instead of address (String) and port (u16) in config #1000

Closed
jhpratt opened this issue May 10, 2019 · 3 comments
Closed
Labels
suggestion A suggestion to change functionality

Comments

@jhpratt
Copy link
Contributor

jhpratt commented May 10, 2019

Is there any particular reason why this isn't being used currently? SocketAddr has been stable since 1.0.0, and allows for parsing/stringifying both IPv4 and IPv6 addresses (including ports). As a bonus, this would slightly reduce the amount of work on Rocket's side, since you wouldn't need to parse it yourself.

Needless to say this would be a breaking change, so 0.5 at the minimum.

@jebrosen
Copy link
Collaborator

#594 also changes address configuration, but in a very different way. I think it's at least worth considering SocketAddr and documenting why we do/don't use it, especially for a hyper 0.12 migration which would change a lot of the low level connection code anyway.

I do like the convenience of ROCKET_PORT=8080, and it might be tricky to support both ideas.

@jebrosen jebrosen added the suggestion A suggestion to change functionality label May 10, 2019
@jhpratt
Copy link
Contributor Author

jhpratt commented May 10, 2019

With regard to ROCKET_PORT, there is a SocketAddr::set_port that takes a u16. It could certainly be documented that environment variables override the inline configuration.

The changeover to hyper 0.12 is a good point, a large portion will likely have to be modified regardless.

@SergioBenitez
Copy link
Member

Let's handle this in #17.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
suggestion A suggestion to change functionality
Projects
None yet
Development

No branches or pull requests

3 participants