[issue32236] open() shouldn't silently ignore buffering=1 in binary mode

[STINNER Victor]

STINNER Victor  added the comment:

I don't think that it would be a good idea to start emitting a new warning in a 
minor release like the future Python 3.7.2, so I suggest to not backport the 
change.

I close the issue.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32236] open() shouldn't silently ignore buffering=1 in binary mode

[STINNER Victor]

STINNER Victor  added the comment:


New changeset a2670565d8f5c502388378aba1fe73023fd8c8d4 by Victor Stinner 
(Alexey Izbyshev) in branch 'master':
bpo-32236: open() emits RuntimeWarning if buffering=1 for binary mode (GH-4842)
https://github.com/python/cpython/commit/a2670565d8f5c502388378aba1fe73023fd8c8d4


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32236] open() shouldn't silently ignore buffering=1 in binary mode

[Alexey Izbyshev]

Alexey Izbyshev  added the comment:

Thank you, Gregory. I didn't intend to add the warning to stable branches -- 
it's just that 3.7 hasn't been released yet when this report was submitted.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32236] open() shouldn't silently ignore buffering=1 in binary mode

[Gregory P. Smith]

Gregory P. Smith  added the comment:

My problem with a warning is the standard one: People who see a warning are 
often end users of python applications (who don't even have to know what Python 
is, let alone know anything about the code).  For that reason, never add a 
warning to a stable branch - only new releases (meaning 3.8 here).

Given that this isn't not a deprecation of meaningful API behavior but is 
highlighting questionably undefined nonsense behavior, users complaining upon 
obtaining 3.8 should ultimately reach library and application developers who 
use the API wrong to update their call sites to explicitly ask for what they 
intended instead of being ambiguious.

FYI - the subprocess.py related changes in your PR are correct.

--
nosy: +gregory.p.smith
versions: +Python 3.8 -Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32236] open() shouldn't silently ignore buffering=1 in binary mode

[STINNER Victor]

Change by STINNER Victor :


--
nosy: +vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32236] open() shouldn't silently ignore buffering=1 in binary mode

[Alexey Izbyshev]

Alexey Izbyshev  added the comment:

Any feedback on the updated PR?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32236] open() shouldn't silently ignore buffering=1 in binary mode

[Alexey Izbyshev]

Alexey Izbyshev  added the comment:

I had similar thoughts when I was fixing tests that broke due to ValueError. 
I've updated the PR to issue a RuntimeWarning instead.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32236] open() shouldn't silently ignore buffering=1 in binary mode

[Antoine Pitrou]

Antoine Pitrou  added the comment:

After looking at the PR, I think it would be a bit too strong to raise an 
error. Perhaps emit a warning instead?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32236] open() shouldn't silently ignore buffering=1 in binary mode

[Alexey Izbyshev]

Change by Alexey Izbyshev :


--
keywords: +patch
pull_requests: +4730
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32236] open() shouldn't silently ignore buffering=1 in binary mode

[Alexey Izbyshev]

Alexey Izbyshev  added the comment:

I'm in favor of raising an exception because it'll expose existing code with 
incorrect assumptions. I'll check whether it breaks any tests and submit a PR.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32236] open() shouldn't silently ignore buffering=1 in binary mode

[Antoine Pitrou]

Antoine Pitrou  added the comment:

> I suggest to raise a ValueError if buffering=1 is used for binary mode.

Either that, or we instead accept buffering=1 as a regular buffer size.  But 
since buffering=1 means something else for text mode, maybe you're right that 
it's better to raise in binary mode, to avoid any possible confusion.

--
nosy: +benjamin.peterson, stutzbach

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32236] open() shouldn't silently ignore buffering=1 in binary mode

[Alexey Izbyshev]

New submission from Alexey Izbyshev :

The fact that "buffering=1" is usable only in text mode is documented for 
open(). In binary mode, setting buffering to 1 is silently ignored and 
equivalent to using default buffer size. I argue that such behavior is:

1. Inconsistent
For example, attempts to use buffering=0 in text mode or to specify encoding in 
binary mode raise ValueError.

2. Dangerous
Consider the following code fragment running on *nix (inspired by real-world 
code):

with open("fifo", "wb", buffering=1) as out:
for line in lines:
out.write(line)

"fifo" refers to a FIFO (named pipe). For each line, len(line) <= PIPE_BUF. 
Because of line buffering, such code must be able to safely assume that each 
write() is atomic, so that even in case of multiple writers no line read by a 
FIFO reader will ever get intermixed with another line. And it's so in Python 
2. After migration to Python 3 (assuming that line is now bytes), this code 
still works on Linux because of two factors:
a) PIPE_BUF is 4096, and so is the default IO buffer size (usually)
b) When a write() is going to overflow the buffer, BufferedWriter first flushes 
and then processes the new write() (based on my experiment). So, each OS 
write() is called with complete lines only and is atomic per (a).

But PIPE_BUF is 512 on Mac OS X (I don't know about default buffer size). So, 
we are likely to have a subtle 2-to-3 migration issue.

I suggest to raise a ValueError if buffering=1 is used for binary mode. Note 
that issue 10344 (msg244354) and issue 21332 would have been uncovered earlier 
if it was done from the beginning.

--
components: IO
messages: 307775
nosy: izbyshev, pitrou
priority: normal
severity: normal
status: open
title: open() shouldn't silently ignore buffering=1 in binary mode
type: behavior
versions: Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com