Expose local_address and remote_address.

[miracle2k]

No description provided.

[coveralls]

Pull Request Test Coverage Report for Build 44

• 2 of 4 (50.0%) changed or added relevant lines in 1 file are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage decreased (-0.5%) to 79.458%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
trio_websocket/init.py	2	4	50.0%

Files with Coverage Reduction	New Missed Lines	%
trio_websocket/init.py	1	87.42%

Totals
Change from base Build 41:	-0.5%
Covered Lines:	352
Relevant Lines:	443

---

💛 - Coveralls

[coveralls]

Pull Request Test Coverage Report for Build 44

• 2 of 4 (50.0%) changed or added relevant lines in 1 file are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage decreased (-0.5%) to 79.458%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
trio_websocket/init.py	2	4	50.0%

Files with Coverage Reduction	New Missed Lines	%
trio_websocket/init.py	1	87.42%

Totals
Change from base Build 41:	-0.5%
Covered Lines:	352
Relevant Lines:	443

---

💛 - Coveralls

[belm0]

Thank you. Would you please add a test?

Is there any argument to be made for just exposing the socket object proper?

[njsmith]

It would make more sense to expose the stream object than to expose the socket object. There isn't always a socket, but there is always a stream. And the stream is useful in other circumstances too, e.g. SSLStream has methods to get information about the peer's certificate.

There's also python-trio/trio#280, which is about giving Stream a high-level API for accessing local and remote addresses... that + exposing the stream on the websocket could potentially replace this PR.

Note that when we do add this API to Stream, it will probably use await to fetch the addresses, and probably won't return bare tuples.

[miracle2k]

Maybe the way forward then is to expose the stream, require the user for now to access websocket.stream.socket.getpeername(), which should remain stable, and later gain the benefit of the new trio APIs that might be available on stream directly.

[mehaase]

Making stream part of the public API feels dangerous. If the caller starts doing things with the raw stream, it could break the WebSocket protocol. For example, the caller could do perform some send or receive on the stream that would cause our local WebSocket state to miss some frames and yield invalid state.

I'd much prefer the public API to have read-only properties like remote address, local address, or SSL certificate. If somebody really needs to manipulate the stream, they can either access the private attribute _stream or they can create their own stream and wrap it in a WebSocket.

[mehaase]

I like this PR but a few requests:

1. Not all streams have a socket, and we shouldn't leak an AttributeError. Returning None seems reasonable.
2. There is a ListenPort class in this file that combines an IP address and a port number and exposes them in a nice little object. Maybe we can reuse that here if it is renamed to something like SocketInfo or something like that.
3. Please add test coverage.

Also,

[mehaase]

This is addressed in #109.