On Mon, Oct 15, 2012 at 05:08:22PM -0000, Jason Conti wrote:
> Review: Approve
>
> Looks good to me overall. My only concern is that test_cache_delete_leasks_fds() may fail in certain circumstances, and it happened to fail in my test rebuild. Python does eventually get around to deleting the records instance.
Thanks a lot! Oh, interessting. I had hoped that the gc.collect()
would ensure that its deleted and thereforce closed. Does the test
fails reliable for you? If so, any hints what I can do trying to
reproduce the issue?
> I like the other additional tests, and actually had a very similar test to test_cache_close_download_fails() but it didn't occur to me to include it for some reason. It also appears I completely missed the case about leaking fds on cache reopen, thanks for fixing it.
On Mon, Oct 15, 2012 at 05:08:22PM -0000, Jason Conti wrote: delete_ leasks_ fds() may fail in certain circumstances, and it happened to fail in my test rebuild. Python does eventually get around to deleting the records instance.
> Review: Approve
>
> Looks good to me overall. My only concern is that test_cache_
Thanks a lot! Oh, interessting. I had hoped that the gc.collect()
would ensure that its deleted and thereforce closed. Does the test
fails reliable for you? If so, any hints what I can do trying to
reproduce the issue?
> I like the other additional tests, and actually had a very similar test to test_cache_ close_download_ fails() but it didn't occur to me to include it for some reason. It also appears I completely missed the case about leaking fds on cache reopen, thanks for fixing it.
Great, thanks for the review!
Cheers,
Michael