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

issue4428 - make io.BufferedWriter observe max_buffer_size limits #49261

Closed
pitrou opened this issue Jan 20, 2009 · 3 comments
Closed

issue4428 - make io.BufferedWriter observe max_buffer_size limits #49261

pitrou opened this issue Jan 20, 2009 · 3 comments

Comments

@pitrou
Copy link
Member

pitrou commented Jan 20, 2009

BPO 5011
Nosy @gpshead, @pitrou

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2009-01-20.11:29:43.998>
created_at = <Date 2009-01-20.11:12:49.990>
labels = ['invalid']
title = 'issue4428 - make io.BufferedWriter observe max_buffer_size limits'
updated_at = <Date 2009-01-20.22:44:41.277>
user = 'https://github.com/pitrou'

bugs.python.org fields:

activity = <Date 2009-01-20.22:44:41.277>
actor = 'gregory.p.smith'
assignee = 'none'
closed = True
closed_date = <Date 2009-01-20.11:29:43.998>
closer = 'pitrou'
components = []
creation = <Date 2009-01-20.11:12:49.990>
creator = 'pitrou'
dependencies = []
files = []
hgrepos = []
issue_num = 5011
keywords = []
message_count = 3.0
messages = ['80242', '80246', '80289']
nosy_count = 2.0
nosy_names = ['gregory.p.smith', 'pitrou']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue5011'
versions = []

@pitrou
Copy link
Member Author

pitrou commented Jan 20, 2009

http://codereview.appspot.com/12470/diff/1/2
File Lib/io.py (right):

http://codereview.appspot.com/12470/diff/1/2#newcode1055
Line 1055: # b is an iterable of ints, it won't always support len().
There is no reason for write() to accept arbitrary iterable of ints,
only bytes-like and buffer-like objects. It will make the code simpler.

http://codereview.appspot.com/12470/diff/1/2#newcode1060
Line 1060: # No buffer API? Make intermediate slice copies instead.
Objects without the buffer API shouldn't be supported at all.

http://codereview.appspot.com/12470/diff/1/2#newcode1066
Line 1066: while chunk and len(self._write_buf) > self.buffer_size:
What if buffer_size == max_buffer_size? Is everything still written ok?

http://codereview.appspot.com/12470/diff/1/2#newcode1070
Line 1070: written += e.characters_written
e.characters_written can include bytes which were already part of the
buffer before write() was called, but the newly raised BlockingIOError
should only count those bytes which were part of the object passed to
write().

http://codereview.appspot.com/12470/diff/1/3
File Lib/test/test_io.py (right):

http://codereview.appspot.com/12470/diff/1/3#newcode496
Line 496: def testWriteNoLengthIterable(self):
This shouldn't work at all. If it works right now, it is only a
side-effect of the implementation.
(it won't work with FileIO, for example)

http://codereview.appspot.com/12470

@pitrou
Copy link
Member Author

pitrou commented Jan 20, 2009

Mmmh, adding <report@bugs.python.org> in the Rietveld CC field didn't
have the expected effect :)

@pitrou pitrou closed this as completed Jan 20, 2009
@pitrou pitrou added the invalid label Jan 20, 2009
@gpshead
Copy link
Member

gpshead commented Jan 20, 2009

Reviewers: Antoine Pitrou,

Message:
Just responding to your comments on the support for generators and non
buffer api supporting inputs.

I'll get to the other comments in the code soon with new unit tests for
those cases.

http://codereview.appspot.com/12470/diff/1/2
File Lib/io.py (right):

http://codereview.appspot.com/12470/diff/1/2#newcode1055
Line 1055: # b is an iterable of ints, it won't always support len().
On 2009/01/20 11:12:47, Antoine Pitrou wrote:

There is no reason for write() to accept arbitrary iterable of ints,
only
bytes-like and buffer-like objects. It will make the code simpler.

Agreed. But since I want to merge this into release30-maint doing that
sounds like a behavior change. I'd be fine with removing it for the
3.1/2.7 version of this code (though I hope people will be using the C
implementation instead).

http://codereview.appspot.com/12470/diff/1/2#newcode1060
Line 1060: # No buffer API? Make intermediate slice copies instead.
On 2009/01/20 11:12:47, Antoine Pitrou wrote:

Objects without the buffer API shouldn't be supported at all.

Same reason as above.

http://codereview.appspot.com/12470/diff/1/3
File Lib/test/test_io.py (right):

http://codereview.appspot.com/12470/diff/1/3#newcode496
Line 496: def testWriteNoLengthIterable(self):
On 2009/01/20 11:12:47, Antoine Pitrou wrote:

This shouldn't work at all. If it works right now, it is only a
side-effect of
the implementation.
(it won't work with FileIO, for example)

hmm in that case it might not be too large of a thing to break when
merged into release30-maint. I'll leave that up to the release manager
& bdfl. my gut feeling for a release branch change is to say we have to
live with this being supported for now.

Description:
http://bugs.python.org/issue4428 - patch gps04.

Please review this at http://codereview.appspot.com/12470

Affected files:
Lib/io.py
Lib/test/test_io.py

Index: Lib/io.py
===================================================================

--- Lib/io.py	(revision 68796)
+++ Lib/io.py	(working copy)
@@ -1047,11 +1047,42 @@
                      self._flush_unlocked()
                  except BlockingIOError as e:
                      # We can't accept anything else.
-                    # XXX Why not just let the exception pass through?
+                    # Reraise this with 0 in the written field as none of  
the
+                    # data passed to this call has been written.
                      raise BlockingIOError(e.errno, e.strerror, 0)
              before = len(self._write_buf)
-            self._write_buf.extend(b)
-            written = len(self._write_buf) - before
+            bytes_to_consume = self.max_buffer_size - before
+            # b is an iterable of ints, it won't always support len().
+            if hasattr(b, '__len__') and len(b) > bytes_to_consume:
+                try:
+                    chunk = memoryview(b)[:bytes_to_consume]
+                except TypeError:
+                    # No buffer API?  Make intermediate slice copies  
instead.
+                    chunk = b[:bytes_to_consume]
+                # Loop over the data, flushing it to the underlying raw IO
+                # stream in self.max_buffer_size chunks.
+                written = 0
+                self._write_buf.extend(chunk)
+                while chunk and len(self._write_buf) > self.buffer_size:
+                    try:
+                        self._flush_unlocked()
+                    except BlockingIOError as e:
+                        written += e.characters_written
+                        raise BlockingIOError(e.errno, e.strerror, written)
+                    written += len(chunk)
+                    assert not self._write_buf, "_write_buf should be  
empty"
+                    if isinstance(chunk, memoryview):
+                        chunk = memoryview(b)[written:
+                                              written +  
self.max_buffer_size]
+                    else:
+                        chunk = b[written:written + self.max_buffer_size]
+                    self._write_buf.extend(chunk)
+            else:
+                # This could go beyond self.max_buffer_size as we don't  
know
+                # the length of b.  The alternative of iterating over it  
one
+                # byte at a time in python would be slow.
+                self._write_buf.extend(b)
+                written = len(self._write_buf) - before
              if len(self._write_buf) > self.buffer_size:
                  try:
                      self._flush_unlocked()
Index: Lib/test/test_io.py

===================================================================
--- Lib/test/test_io.py (revision 68796)
+++ Lib/test/test_io.py (working copy)
@@ -479,6 +479,33 @@

          self.assertEquals(b"abcdefghijkl", writer._write_stack[0])

+ def testWriteMaxBufferSize(self):
+ writer = MockRawIO()
+ bufio = io.BufferedWriter(writer, buffer_size=8,
max_buffer_size=13)
+
+ bufio.write(b"abcdefghij")
+ bufio.write(b"klmnopqrstuvwxyz")
+ bufio.write(b"0123456789"*3)
+
+ expected_writes = [b"abcdefghij", b"klmnopqrstuvw",
b"xyz0123456789",
+ b"0123456789012"]
+ self.assertEquals(expected_writes, writer._write_stack)
+ bufio.close()
+ self.assertEquals(b"3456789", writer._write_stack[4])
+
+ def testWriteNoLengthIterable(self):
+ writer = MockRawIO()
+ bufio = io.BufferedWriter(writer, buffer_size=8)
+
+ def Generator(value):
+ for x in value:
+ yield x
+ iterable = Generator(b'ABCDEFGHIJKLMNOPQRSTUVWXYZ')
+ self.assertRaises(TypeError, len, iterable)
+ bufio.write(iterable)
+
+ self.assertEquals(b'ABCDEFGHIJKLMNOPQRSTUVWXYZ',
writer._write_stack[0])
+
def testWriteNonBlocking(self):
raw = MockNonBlockWriterIO((9, 2, 22, -6, 10, 12, 12))
bufio = io.BufferedWriter(raw, 8, 16)

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 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