-
Notifications
You must be signed in to change notification settings - Fork 0
Add server listening address property in NIOHTTPServer #32
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
Conversation
Currently, when specifying port 0 in `HTTPServerConfiguration`, there is no public API to determine what port the `NIOHTTPServer` has actually been bound to. Modifications: - Added a `SocketAddress` type for representing a host address and port. - Added an async `listeningAddress` property on `NIOHTTPServer` that returns a `SocketAddress` once the server has started listening (or throws an error if the address couldn't be bound to or if the server has closed). - Added a `State` type to `NIOHTTPServer` for representing the idle, listening, and closed states and managing transitions between them. Result: Users can determine the address and port the `NIOHTTPServer` is listening at.
| import NIOCore | ||
|
|
||
| /// Represents an IPv4 address. | ||
| public struct IPv4: Hashable, Sendable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a bit hesitant to add these types to this package. This is probably something that is generally useful and might require some API design on the HTTPServer protocol over in the other repo. WDYT @gjcairo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I don't think this should be part of the NIO implementation only. So yes it would make sense to move them to the other repo. Would you be okay with having these here for now, and I can open a PR on the other repo as well? Then once we land them there we can remove them from here alongside the other abstract pieces.
Also: would SocketAddress be something that's part of the currency types we'd like to have?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it is okay to land them here for now but let's file an issue on the other repo. Ideally SocketAddress or something similar is a shared currency type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| import NIOCore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need the NIO import here right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I missed this. Fixed in #34.
Motivation:
Currently, when specifying port 0 in
HTTPServerConfiguration, there is no API to determine what port theNIOHTTPServerhas actually been bound to.Modifications:
SocketAddresstype for representing a host address and port.listeningAddressproperty onNIOHTTPServerthat returns aSocketAddressonce the server has started listening (or throws an error if the address couldn't be bound to or if the server has closed).Statetype toNIOHTTPServerfor representing the idle, listening, and closed states and managing transitions between them.Result:
Users can determine the address and port the
NIOHTTPServeris listening at.