← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] lp:~wallyworld/launchpad/ppa-packages-timeout-1071581 into lp:launchpad

 

I am going to scrap this work as is - it turns out there will still be 
performance issues for some users, those who will have many 1000s of 
records to be displayed. I was of the assumption that the number of 
records would normally be much smaller than that.

So the plan now is to introduce a new denormalised table, designed for 
reporting, and maintain it with a frequent garbo job, since the data 
does not have to be immediately current.

On Wed 31 Oct 2012 23:24:23 EST, Curtis Hovey wrote:
> Review: Needs Information code
>
> Line 123 looks inconsistent because the storm column does not use ID
>     SourcePackagePublishingHistory.sourcepackagereleaseID
>
> The while-loop looks odd. I don't like use the done variable, but I see you wanted something separate from max_results. I don't understand what happens when max_results is None or 0 -- they seem to lead to contradictory behaviour. I suppose 0 is really a value error for max_results. What happens when max_results is None and the the last batch is returns less than the batch size? eg. rs has only 15 more items, it iterates over them, then back to the start of the loop to get the next batch using a slice that exceeds everything in the rs.

-- 
https://code.launchpad.net/~wallyworld/launchpad/ppa-packages-timeout-1071581/+merge/132236
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.


References