[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