Feature/wrap socket #878
Feature/wrap socket #878
Conversation
|
I dont know why mypy is complaining here because I just copied the call from the |
… into feature/wrap-socket
src/anyio/_backends/_asyncio.py
Outdated
| ) | ||
| return ConnectedUDPSocket(transport, protocol) | ||
| elif raw_socket.type == socket.SocketKind.SOCK_STREAM: | ||
| transport, protocol = cast( # type: ignore[assignment] # mypy is not able to properly infer the type on unix |
There was a problem hiding this comment.
this is because tranport and protocol are already declared on line 2654
will it work if you use unpacking directly into the constructor?
SocketStream(
*(
await get_running_loop().create_connection(
StreamProtocol,
sock=raw_socket,
)
)
)or you could use different variable names, eg:dgram_transport, dgram_protocol = ... and stream_transport, stream_protocol = ...
tests/test_sockets.py
Outdated
| @@ -1526,6 +1530,65 @@ async def test_send_after_close(self, family: AnyIPAddressFamily) -> None: | |||
| await udp.send(b"foo") | |||
|
|
|||
|
|
|||
| @pytest.mark.network | |||
| @pytest.mark.usefixtures("check_asyncio_bug") | |||
There was a problem hiding this comment.
apparently this is fixed in python/cpython#83329
There was a problem hiding this comment.
ah it's not actually fixed, I tried it out
There was a problem hiding this comment.
but the tests pass, so should I add it again or not?
There was a problem hiding this comment.
ah probably because you're not sending any data
There was a problem hiding this comment.
That doesn't explain why all the tests pass.
|
Before you go any further, I must point out that the notion of "client" UDP socket being always connected is fundamentally wrong, and the same function should not be used to wrap either a TCP socket or a UDP socket either. The notion of "connection" for a UDP socket just means that it has a default target address. An unconnected UDP socket could just as well be a client or a server socket. |
What do you suggest then? I picked up the suggested interface from #845 |
Well, that suggestion was also wrong then. I have to think about this for a little bit. Perhaps class methods in the respective socket stream/listener classes would do it? At any rate, let's reach a consensus before proceeding any further. |
that might be a good idea. something like class SocketStream(ByteStream, _SocketProvider):
"""
Transports bytes over a socket.
Supports all relevant extra attributes from :class:`~SocketAttribute`.
"""
@classmethod
@abstractmethod
def from_socket(cls, raw_socket: socket.socket) -> SocketStream: ...What do you think? Or should we include a hint to stdlib sockets like @classmethod
@abstractmethod
def from_stdlib_socket(cls, stdlib_socket: socket.socket) -> SocketStream: ...? |
|
We can't add new abstract methods to public classes as that would break compatibility for anyone implementing their own |
|
I'll have to think about this some more when I get the time and I'm not working on my day job. |
Have you come to any conclusion yet? |
|
Not yet, sorry. |
|
I'll try to work on this issue this weekend. |
Changes
Fixes #845
Checklist
If this is a user-facing code change, like a bugfix or a new feature, please ensure that
you've fulfilled the following conditions (where applicable):
tests/) added which would fail without your patchdocs/, in case of behavior changes or newfeatures)
docs/versionhistory.rst).If this is a trivial change, like a typo fix or a code reformatting, then you can ignore
these instructions.
Updating the changelog
If there are no entries after the last release, use
**UNRELEASED**as the version.If, say, your patch fixes issue #123, the entry should look like this:
If there's no issue linked, just link to your pull request instead by updating the
changelog after you've created the PR.