[MERGE] [Bug 215426] Re: MemoryError - recv for HTTPthroughProxy

Eric Holmberg eholmberg at arrow.com
Wed May 21 22:07:55 BST 2008


Any idea when this patch will be voted on?

http://bundlebuggy.aaronbentley.com/request/%3C6910002A52F85D46AF3E35ED5
B254FE10417E6D0 at wmhex005p.arrownao.corp.arrow.com%3E

Thanks,

Eric

> -----Original Message-----
> From: Eric Holmberg 
> Sent: Tuesday, May 06, 2008 10:41 AM
> To: bazaar at lists.canonical.com
> Cc: Vincent Ladeuil; Andrew Bennetts; John Arbash Meinel; 
> 'Aaron Bentley'
> Subject: RE: [MERGE] [Bug 215426] Re: MemoryError - recv for 
> HTTPthroughProxy
> 
> > 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
>  
> 
> 
> 



More information about the bazaar mailing list