gh-109523: Reading text from a non-blocking stream with read may now raise a BlockingIOError if the operation cannot immediately return bytes. #122933
Conversation
- Introduced `test_read_non_blocking` to ensure proper handling of non-blocking reads using `os.pipe()`. - The test verifies that a `BlockingIOError` is raised when attempting to read from a non-blocking pipe with no data. - Updated code to raise `BlockingIOError` with a specific message when `read()` does not return bytes.
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
There was a problem hiding this comment.
The code in _pyio's implementation of TextIO should ideally be changed in this PR as well so the two main implementations stay in sync (I think, in specific the code around https://github.com/python/cpython/blob/main/Lib/_pyio.py#L2288-L2293, when updating the test to use self.io.open the test I think will fail on PyIO which should help validate where it should go to match behavior)
Definitely liking the direction, looking forward to the next iteration
…dling of BlockingIOError in non-blocking I/O scenarios
…) to ensure the test runs under both _io and _pyio implementations. Also, explicitly specified 'rt' mode in the open() call for clarity, as the test is focused on TextIO behavior.
…n `buffer.read()` returns zero bytes. - Fix double backticks in the documentation.
There was a problem hiding this comment.
Overall I think all the code is correct. I have some personal stylistic nitpicks (How I program / what I'd match doesn't match what you did, but I don't have any strong argument to leave or change, take them or leave them as you wish). Think this needs a more experienced Python reviewer than me at this point.
Thanks a lot for the improvements!
Misc/NEWS.d/next/C_API/2024-08-12-10-15-19.gh-issue-109523.S2c3fi.rst
Outdated
Show resolved
Hide resolved
…IOError change Revised the NEWS entry in `MISC/NEWS.d/next/C_API` to specify that the recent change affecting the `BlockingIOError` now applies to both `_io.TextIOWrapper.read()` and `_pyio.TextIOWrapper.read()`.
- Adapted documentation to reflect common Python style practices. - Refactored code to use a clearer variable name.
read may now raise a :exc:BlockingIOError if the operation cannot immediately return bytes.
read may now raise a :exc:BlockingIOError if the operation cannot immediately return bytes.|
Just dropping a friendly ping here. This pull request is in good shape and has undergone intensive review. Could a maintainer kindly take a look for final review and, if everything looks good, consider merging it? |
|
Also a trivial question from my side. Would this change qualify for an entry in Misc/ACKS? I'd be happy to add it if appropriate. |
|
Simce it's a user facing change I'd say it warrants both a NEWS and a What's New entry to be on the safe side. You can add yourself to ACKS I think (sorry I only read the Misc part and not the file...). Technically, every contributor is free to add themselves there (or am I wrong? I know I've never added myself due to laziness). |
Thanks for the feedback. I have the news entry, but I'm not sure how to handle the "What's New entry". How am I supposed to write one? |
|
Look at the 3.14.rst file for example. The What's New entry should looks like io
--
* Blabla
(Contributed by YOU in :gh:`ISSUE_NUMBER`.)The blabla text is usually the same as the one in the blurb. |
Hi Benedikt, Doc/wathsnew/3.X.rst |
|
The 3.14.rst file. The next.rst file is for the full changelog and is managed automatically. |
Added entry in ACKS.
Fix missing indents and other syntax issues. Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
…locking stream cannot immediately return bytes. (pythonGH-122933)
…locking stream cannot immediately return bytes. (pythonGH-122933)
_io_TextIOWrapper_read_implto raise aBlockingIOErrorwhenread()does not return bytes.test_read_non_blockingto ensure that aBlockingIOErroris raised when attempting to read from a non-blocking pipe with no data usingos.pipe().