[MERGE] [Bug 215426] Re: MemoryError - recv for HTTPthroughProxy
Eric Holmberg
eholmberg at arrow.com
Tue May 6 17:41:16 BST 2008
> Overall, this looks good. But the lack of tests for
> os.pumpfile was a dealbreaker. There are also some minor
> style issues.
I've moved the tests to the os_utils.py module and written a new test to
verify that reads are not exceeding the block size.
> Actually, I would prefer if you added a size parameter to pumpfile.
> This would allow you you pull all the logic out of
> read_limited, and would make pumpfile more generally useful.
>
> But you have changed pumpfile's functionality, so you must
> add some tests for that new functionality.
>
> I would recommend:
> 1. add a size parameter to os.pumpfile
> 2. call os.pumpfile directly from read and remove
> _read_limited 3. change your tests to test pumpfile. You can
> test one edge case of read to ensure pumpfile is being used,
> but you don't need to test it all over again.
Done.
> Please put a space after all commas.
I've cleaned up (hopefully) all of the style issues.
Note that running bzr cdiff --check-style returns a spurious whitespace
change, but that is due to cleaning up trailing whitespace after a
newline.
Let's hope that this is the final revision!
Regards,
Eric Holmberg
Applications Engineer, Arrow Electronics
Engineering Solutions Center
Denver, Colorado
ESC: 877-ESC-8950, esc at arrow.com
Direct: 303-824-4537, eholmberg at arrow.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bugfix215426-v4.patch
Type: application/octet-stream
Size: 19669 bytes
Desc: bugfix215426-v4.patch
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20080506/56caf228/attachment-0001.obj
More information about the bazaar
mailing list