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