[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