-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
bpo-32568: make select.epoll() and its docs consistent #7840
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
* 'flags' is indeed deprecated, but there was a validation on its value, contrary to the docs saying it is "completely ignored". This removes that check. * 'sizehint' is still used when 'epoll_create1()' is unavailable, so this adds mention of this in the docs. * Make 'sizehint' accept -1 again, which is replaced with FD_SETSIZE-1. This is needed to have a default value available at the Python level allowing to set this, since FD_SETSIZE is not exposed to Python. (see: bpo-31938)
vstinner
left a comment
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 don't think that it's correct to ignore flags. I prefer to keep the check rejecting invallid values like flags=12345.
I'm not sure why Python wants to reject unknown flags: can't we rely on the kernel/libc to validate flags?
| @@ -0,0 +1 @@ | |||
| make select.epoll() and its docs consistent re. *sizehint* and *flags* | |||
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.
Please avoid abbreviation, english is not my first language and I don't know what "re." means. Add also an uppercase to "Make".
|
When you're done making the requested changes, leave the comment: |
Other than that value check, the existing code completely ignores the |
|
I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @vstinner: please review the changes made to this pull request. |
|
The argument passed to And I think it is better to keep the check for flags. This can help to backport Python programs to older versions (the same reason as keeping the check for the argument of In future we will get rid of both parameters. Maybe will start producing a deprecation warning in 3.8. |
Also update tests accordingly.
|
@serhiy-storchaka, this is ready for another review. |
serhiy-storchaka
left a comment
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.
Thanks @taleinat!
|
@taleinat: Please replace |
|
Thanks @taleinat for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7. |
|
GH-8024 is a backport of this pull request to the 3.7 branch. |
* `flags` is indeed deprecated, but there is a validation on its value for backwards compatibility reasons. This adds mention of this in the docs. * The docs say that `sizehint` is deprecated and ignored, but it is still used when `epoll_create1()` is unavailable. This adds mention of this in the docs. * `sizehint=-1` is acceptable again, and is replaced with `FD_SETSIZE-1`. This is needed to have a default value available at the Python level, since `FD_SETSIZE` is not exposed to Python. (see: bpo-31938) * Reject `sizehint=0` since it is invalid to pass on to `epoll_create()`. The relevant tests have also been updated. (cherry picked from commit 0cdf5f4) Co-authored-by: Tal Einat <[email protected]>
|
Sorry, @taleinat, I could not cleanly backport this to |
…H-7840) * `flags` is indeed deprecated, but there is a validation on its value for backwards compatibility reasons. This adds mention of this in the docs. * The docs say that `sizehint` is deprecated and ignored, but it is still used when `epoll_create1()` is unavailable. This adds mention of this in the docs. * `sizehint=-1` is acceptable again, and is replaced with `FD_SETSIZE-1`. This is needed to have a default value available at the Python level, since `FD_SETSIZE` is not exposed to Python. (see: bpo-31938) * Reject `sizehint=0` since it is invalid to pass on to `epoll_create()`. The relevant tests have also been updated.. (cherry picked from commit 0cdf5f4) Co-authored-by: Tal Einat <[email protected]>
|
GH-8025 is a backport of this pull request to the 3.6 branch. |
…8024) * `flags` is indeed deprecated, but there is a validation on its value for backwards compatibility reasons. This adds mention of this in the docs. * The docs say that `sizehint` is deprecated and ignored, but it is still used when `epoll_create1()` is unavailable. This adds mention of this in the docs. * `sizehint=-1` is acceptable again, and is replaced with `FD_SETSIZE-1`. This is needed to have a default value available at the Python level, since `FD_SETSIZE` is not exposed to Python. (see: bpo-31938) * Reject `sizehint=0` since it is invalid to pass on to `epoll_create()`. The relevant tests have also been updated. (cherry picked from commit 0cdf5f4) Co-authored-by: Tal Einat <[email protected]>
GH-8025) * `flags` is indeed deprecated, but there is a validation on its value for backwards compatibility reasons. This adds mention of this in the docs. * The docs say that `sizehint` is deprecated and ignored, but it is still used when `epoll_create1()` is unavailable. This adds mention of this in the docs. * `sizehint=-1` is acceptable again, and is replaced with `FD_SETSIZE-1`. This is needed to have a default value available at the Python level, since `FD_SETSIZE` is not exposed to Python. (see: bpo-31938) * Reject `sizehint=0` since it is invalid to pass on to `epoll_create()`. The relevant tests have also been updated. (cherry picked from commit 0cdf5f4)
The docs for
select.epoll()are currently out of sync with the implementation with regard to thesizehintandflagsparameters.flagsis indeed deprecated, but there was a validation on its value, contrary to the docs saying it is "completely ignored". This removes that check.sizehintis still used whenepoll_create1()is unavailable, so this adds mention of this in the docs.sizehintaccept-1again, which is replaced withFD_SETSIZE-1. This is needed to have a default value available at the Python level allowing to set this, sinceFD_SETSIZEis not exposed to Python. (see: bpo-31938)https://bugs.python.org/issue32568