[00:04] wgrant: With Component and Section preloaded, down to 70. [00:09] Excellent [00:09] What are the rest? [00:12] wgrant: PackageDiff and related LFA and LFC; SPPH; ArchivePermission [00:13] Right [00:18] wgrant: So I think it's reasonable [00:18] We've dropped 160 queries [00:52] wgrant: http://pastebin.ubuntu.com/1453695/ [01:15] wgrant: No thoughts at all, I see. [01:26] StevenK: Hmm [01:26] Looks reasonable so far [01:30] wgrant: Hmmm, given IPackageUploadSet.getAll() is the meat inside IDistroSeries.getPackageUploads(), and getAll() does the propertycache dance, I guess the other bug is sorted too? [01:31] StevenK: It probably is, but you need to check that the delegations to SPR's checker work [02:08] (by "work" I mean "don't issue tonnes of queries") [03:31] wgrant: Hmmm, something has broken the upload webservice tests [03:43] Oh? [03:44] wgrant: TestPackageUploadWebservice calls factory.makeBuildPackageUpload() (and then creates a BPR, but the factory call does that ...), trying to access upload.builds[0].build leads to an IndexError [03:45] StevenK: Sounds like something's creating a PUB without using the method that knows to invalidate stuff [03:46] But it uses addBuild, which does the invalidation! [03:46] def addBuild(self, build): [03:46] """See `IPackageUpload`.""" [03:46] del get_property_cache(self).builds [03:46] It's probably clearing the cache too early [03:48] We should delay clearing the property cache? [03:49] Clear the cache once the relevant change has been made [03:49] Otherwise the cache may be recalculated with the old data [03:49] Then all three of the methods are wrong [03:49] Lots of code is wrong [03:50] Most code is wrong [03:50] Oh, they used to del get_property_cache, return, the searchable_{name,version}s stuff is now in the middle [03:55] wgrant: I moved the del to just before the return, but it still dies. Guess I need to create the PUB, delete the property cache and then return [03:57] StevenK: Right, that's what I meant. [03:58] It's not necessarily safe to clear the cache before you've made the change [03:58] Because some operation (eg. accessing a protected attribute) may cause the cache to be recalculated [03:58] At any time [03:58] print upload.builds[0].build [03:58] IndexError: list index out of range [03:58] :-( [04:06] Do not get it. We create the upload in the factory, create a BPB and a BPR, add it to the PU, where we destroy the cached property, and then return it, and the test says upload.builds is [] [04:09] Give me a branch :) [04:09] Or diff [04:10] wgrant: http://pastebin.ubuntu.com/1453948/ [04:18] StevenK: Looking [04:19] Hm [04:19] StevenK: I wonder if it's not flushing properly [04:22] wgrant: I was thinking about that -- something that is more aggressive about caching for tests === tumbleweed_ is now known as tumbleweed [04:26] An explicit flush fixes it [04:26] Odd [04:26] wgrant: As in a store flush? [04:27] Yes [04:28] It looks like the SQLMultipleJoin isn't causing a flush [04:28] Shall I rewrite them into properties that do a IStore.find() ? [04:28] No [04:28] Since SQLMultipleJoin annoys me [04:28] We should work out why it's not working === broder__ is now known as broder [04:30] Oh [04:30] of course [04:30] checkPermission is wrapped in @block_implicit_flushes === broder is now known as broder___ [04:31] I'd explicitly flush before invalidating the cache in each case === broder___ is now known as broder [04:31] Right [04:33] Ah ha, excellent [04:34] wgrant: Shall I change that print to a assert just to check that this doesn't bite again? [04:34] StevenK: No [04:34] Because it will crash if it fails [04:34] Already [04:34] Won't it? [04:34] No, because I stopped creating a BPR [04:35] And then the method just iterates over builds, which won't care if it's empty [04:35] Ah [04:35] So I can revert the change to create another BPR, which I think is silly, but does test that builds is usable, or an assert [04:41] Haha, one test depends on the second BPR [04:41] Eh, back it goes [04:46] wgrant: So you'd like another test to check query counts? [04:49] Please [04:49] wgrant: For IDistroSeries.getPackageUploads() via the Webservice or what? [04:50] That's what I'd do [04:51] Given that it's going to be lazr.restful triggering the launchpad.LimitedView checks [05:04] wgrant: http://pastebin.ubuntu.com/1453995/ [05:09] StevenK: What are those two adjacent SPPH queries? [05:10] Identical. Odd. [05:12] * StevenK tries to work out the backtrace [05:19] wgrant: Do not get it [05:20] Also not helped by the backtrace being on one line [05:36] Gah [05:36] Can't even convert it to something readable [08:25] When does patch-2209-40-3.sql get applied? I merged db-devel when I shouldn't have === almaisan-away is now known as al-maisan === yofel_ is now known as yofel === Ursinha is now known as Ursinha-afk === Ursinha-afk is now known as Ursinha === al-maisan is now known as almaisan-away === daker_ is now known as daker === mthaddon is now known as frogman === teknico_ is now known as teknico [16:44] Whoa, I'd forgotten Launchpad used to have "mentorships" ... That's a blast from the past === gary_poster is now known as gary_poster|away === gary_poster|away is now known as gary_poster === almaisan-away is now known as al-maisan === al-maisan is now known as almaisan-away === gary_poster is now known as gary_poster|away === gary_poster|away is now known as gary_poster === gary_poster is now known as gary_poster|away