-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 poll() instead of select() #2279
base: main
Are you sure you want to change the base?
Conversation
Please sign your commits following these rules: $ git clone -b "master" [email protected]:bieron/docker-py.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -f Amending updates the existing PR. You DO NOT need to open a new one. |
Unfortunately, |
You're correct about lack of select.poll. It's not only windows, but also using eventlet (e.g. in celery) magically patches your select module, removing poll from it. |
This PR is not passing due to an already solved problem. |
Fixes docker#2278 because poll does not have select's builtin limit of 1024 descriptors Signed-off-by: Jan Bieron <[email protected]>
Fixes docker#2278, which was originally addressed in docker#2279, but was not properly merged. Additionally it did not address the problem of poll not existing on Windows. This patch falls back on the more limited select method if host system is Windows. Signed-off-by: Tyler Westland <[email protected]>
Hi, just wanted to check in with this as I encounter the same issue. What is the status on this?
|
Fixes #2278, which was originally addressed in #2279, but was not properly merged. Additionally it did not address the problem of poll not existing on Windows. This patch falls back on the more limited select method if host system is Windows. Signed-off-by: Tyler Westland <[email protected]>
…er-py#2865) Fixes docker/docker-py#2278, which was originally addressed in docker/docker-py#2279, but was not properly merged. Additionally it did not address the problem of poll not existing on Windows. This patch falls back on the more limited select method if host system is Windows. Cherry-picked from docker/docker-py@a02ba74 Co-authored-by: Tyler Westland <[email protected]>
* socket: fix for errors on pipe close in Windows (docker/docker-py#3099) Need to return data, not size. By returning an empty string, EOF will be detected properly since `len()` will be `0`. Fixes docker/docker-py#3098. Cherry-picked from docker/docker-py@f846232 Co-authored-by: Milas Bowman <[email protected]> * socket: use poll() instead of select() except on Windows (docker/docker-py#2865) Fixes docker/docker-py#2278, which was originally addressed in docker/docker-py#2279, but was not properly merged. Additionally it did not address the problem of poll not existing on Windows. This patch falls back on the more limited select method if host system is Windows. Cherry-picked from docker/docker-py@a02ba74 Co-authored-by: Tyler Westland <[email protected]> * api: respect timeouts on Windows named pipes (docker/docker-py#3112) Cherry-picked from docker/docker-py@9cadad0 Co-authored-by: Imogen <[email protected]> * Add URL to changelog. * api: avoid socket timeouts when executing commands (docker/docker-py#3125) Only listen to read events when polling a socket in order to avoid incorrectly trying to read from a socket that is not actually ready. Cherry-picked from docker/docker-py@c5e582c Co-authored-by: Loïc Leyendecker <[email protected]> --------- Co-authored-by: Milas Bowman <[email protected]> Co-authored-by: Tyler Westland <[email protected]> Co-authored-by: Imogen <[email protected]> Co-authored-by: Loïc Leyendecker <[email protected]>
Fixes #2278
because poll does not have select's builtin limit of 1024 descriptors
This pr does not contain any updates to tests although it probably should. I'm also not 100% sure it is a proper fix, but it does solve the issue of #2278 when tested manually.