Code review comment for lp:~jaypipes/glance/bug700162

Revision history for this message
Jay Pipes (jaypipes) wrote :

On Fri, Jan 21, 2011 at 12:00 AM, Rick Harris <email address hidden> wrote:
> Review: Needs Fixing
> Perfect timing, hit this bug just now and there is already a patch :)
>
> Overall, your fix looks really good.  I had to make a couple of small tweaks to get it fully-working. My fixes are here:
>
> https://code.launchpad.net/~rconradharris/glance/bug700162

Swett, thanks Rick! :) I'll merge your patch into mine and push shortly.

>> Not sure how we fully test this, since we'd have to pack up
>> a very large file for use in testing, but I'm open to suggestion
>
> I just tested the code functionally by uploading ubuntu-lucid into Glance. Without the fix I get a MemoryError, with it, it uploads successfully.

Great! I like Ewan's idea for testing below; I'll try to integrate
that idea in too.

-jay

« Back to merge proposal