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

make wait_socket_{readable,writable}() accept a file descriptor #386

Closed
dlax opened this issue Nov 9, 2021 · 12 comments
Closed

make wait_socket_{readable,writable}() accept a file descriptor #386

dlax opened this issue Nov 9, 2021 · 12 comments

Comments

@dlax
Copy link

dlax commented Nov 9, 2021

Both asyncio's add_reader/writer() and trio's wait_{readable,writable}() accept a file descriptor argument along with any object with a fileno() method (not just a socket.socket).
Can we make anyio support a similar interface?
(Perhaps following trio's names and drop the _socket part from functions name?)

Happy to work on a PR if agreed.

@agronholm
Copy link
Owner

I don't really encourage the use of these functions as they don't work on Windows. What use case did you have in mind?

@dlax
Copy link
Author

dlax commented Nov 9, 2021

I'm trying to see if psycopg could work with anyio, see psycopg/psycopg#29 and discussions in psycopg/psycopg2#1057. (I am aware of the limitations.)
Basically, we have a PGconn object wrapping a libpq connection and we need to wait for the underlying file descriptor to become read- or write-ready (https://www.postgresql.org/docs/current/libpq-async.html).

@agronholm
Copy link
Owner

What would you pass to these functions if not a socket.socket object?

@agronholm
Copy link
Owner

Does PGconn have a fileno() method?

@dlax
Copy link
Author

dlax commented Nov 9, 2021

I'd pass a fd: int, which trio and asyncio functions already accept.
PGconn has a socket property (mapping PQsocket accessor), this returns a file descriptor number.

@dlax
Copy link
Author

dlax commented Nov 9, 2021

In fact socket.socket is already too restrictive; probably a HasFileno protocol as defined in typeshed (https://github.com/python/typeshed/blob/994b69ef8f18e76689daca3947879c3d7f76173e/stdlib/_typeshed/__init__.pyi#L149) would be enough.

@agronholm
Copy link
Owner

HasFileno was too broad for functions designed to accept only sockets (they do work with selector loops on Windows, just not with proactor loops).

@agronholm
Copy link
Owner

So as not to change the scope of the functions, can we easily check that a file descriptor refers to a socket and not another type of file?

@agronholm
Copy link
Owner

We could let the back-end do that but I would like to keep the resulting exception consistent.

@dlax
Copy link
Author

dlax commented Nov 9, 2021

I don't know in general.
For the libpq connection, I tried that socket.socket(fileno=pgconn.socket) but this fails with OSError: [Errno 9] Bad file descriptor.

@agronholm
Copy link
Owner

Could you not just use socket.fromfd()?

@dlax
Copy link
Author

dlax commented Nov 9, 2021

socket.fromfd() works better; thank you!

dlax added a commit to dlax/psycopg that referenced this issue Nov 10, 2021
We need to build a socket object in these functions in order to use
anyio.wait_socket_{readable,writable}(). See discussions at
agronholm/anyio#386
Perhaps we should maintain a cache of these for performance?
dlax added a commit to dlax/psycopg that referenced this issue Nov 11, 2021
We need to build a socket object in these functions in order to use
anyio.wait_socket_{readable,writable}(). See discussions at
agronholm/anyio#386
Perhaps we should maintain a cache of these for performance?
dlax added a commit to dlax/psycopg that referenced this issue Nov 17, 2021
We need to build a socket object in these functions in order to use
anyio.wait_socket_{readable,writable}(). See discussions at
agronholm/anyio#386
Perhaps we should maintain a cache of these for performance?
dlax added a commit to dlax/psycopg that referenced this issue Nov 18, 2021
We need to build a socket object in these functions in order to use
anyio.wait_socket_{readable,writable}(). See discussions at
agronholm/anyio#386
Perhaps we should maintain a cache of these for performance?
dlax added a commit to dlax/psycopg that referenced this issue Nov 18, 2021
We need to build a socket object in these functions in order to use
anyio.wait_socket_{readable,writable}(). See discussions at
agronholm/anyio#386
Perhaps we should maintain a cache of these for performance?
dlax added a commit to dlax/psycopg that referenced this issue Nov 19, 2021
XXX: tests fail!

We need to build a socket object in these functions in order to use
anyio.wait_socket_{readable,writable}(). See discussions at
agronholm/anyio#386
dlax added a commit to dlax/psycopg that referenced this issue Nov 20, 2021
We need to build a socket object in these functions in order to use
anyio.wait_socket_{readable,writable}(). See discussions at
agronholm/anyio#386

In tests, we define 'wait_{conn_,}_async' fixtures that will pick either
asyncio or anyio waiting functions depending on the value of
'anyio_backend' parametrized fixture (which will be either 'asyncio' and
'trio').
dlax added a commit to dlax/psycopg that referenced this issue Nov 20, 2021
We need to build a socket object in these functions in order to use
anyio.wait_socket_{readable,writable}(). See discussions at
agronholm/anyio#386

In tests, we define 'wait_{conn_,}_async' fixtures that will pick either
asyncio or anyio waiting functions depending on the value of
'anyio_backend' parametrized fixture (which will be either 'asyncio' and
'trio').
@agronholm agronholm closed this as not planned Won't fix, can't repro, duplicate, stale Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants