=== slank is now known as slank_away [02:52] wgrant: Can you poke at http://pastebin.ubuntu.com/1451410/ and see if you can make sense of why my preloading changes don't seem to help at all? [03:23] StevenK: What's not working? [03:24] wgrant: It seems to me like the preloading PackageDiff call isn't working at all, since the code still loops loading PackageDiffs one by one [03:25] Well, that's not really surprising [03:25] How would that work? [03:25] The PK is ID, not (from, to) [03:25] So simply loading them into the Storm cache isn't going to do much good [03:25] As the view is going to be querying by (from, to) [03:26] wgrant: Eh? I'm using getDiffsToReleases() which should okay, no? [03:26] Sure, that'll load the right ones [03:26] But how will subsequent calls know that they're already loaded? [03:26] How will they find them? [03:28] wgrant: What's more concerning is I can't even see the preloading query in the OOPS [03:33] StevenK: It's there for me [03:33] I see the initial diff grabbing the actual packagediff rows [03:33] s/diff/query/ [03:33] Then a subsequent query looking for the existence of a packagediff for the single source [03:33] So it all seems fine [03:37] Question guys.. the bugs I fixed are starting to have qa-needstesting, am I allowed to test them? if so, what's the process for testing them? [03:38] cjohnston: Try it on qastaging [03:38] Just check that they don't obviously break anything on qastaging.launchpad.net [03:38] If the fix is good, change qa-needstesting to qa-ok [03:39] ok.. cool.. wasn't sure if as the code fixer I was allowed to do the reviews [03:39] We normally QA our own stuff if we're around [03:40] ok.. anyone know what the URL is to create a new distro? [03:40] I think you need to be an admin for that. [03:41] Ah, yeah, normal users can't create them [03:41] do you two have admin access? [03:41] No [03:42] I can't test line 46 then https://code.launchpad.net/~chrisjohnston/launchpad/181725/+merge/140653 [03:43] I'm not sure that such a trivial change warrants the effort of obtaining admin privileges [03:43] The test suite is a pretty good indication that it works fine [03:43] ok.. the public views look fine.. so I can mark it qa-ok? [03:43] Right [03:43] its just a text change anyway [03:43] ok..thanks wgrant and StevenK [03:44] cjohnston: You're welcome [03:44] wgrant: Hmm, yeah. Now I see the preload [03:44] 4 coomits in one week.. thats a record for me.. by about 3 [03:45] Heh, I don't track how many commits I do [03:47] StevenK: your kind-of on the LP team... I'm community :-P [03:51] So it appears my attempts at preloading to drop the query count don't really help. [03:53] I take that back. Preloading changesfile drops from 237 to 213. [03:59] * StevenK stabs zope.tal [04:00] It looks like the tal has a condition on not: packagediff or so, and that's forcing storm to grab each packagediff, LFA and then LFC [04:10] StevenK: I'd expect a packagediff existence check, then a packagediff retrieval query, then LFA and LFC queries for each diff [04:11] Yeah, I'm trying to staple packagediff into CPU so I can stop those [04:13] Why it chains through to LFA and LFC, though ... [04:13] package_diffs is a SQLMultipleJoin, and the TAL seems to only want a fmt:link [04:14] Why wouldn't that want LFA and LFC? [04:14] It needs LFA for the link [04:14] And LFC for the size [05:04] wgrant: 237 down to 186. [05:06] Getting there [05:07] Now 167 [05:08] (214-186: Preloading PUBs, BPBs and binary SPRs, 186-167: Preloading all SPNs for source and binary SPRs) [05:11] 153, preloading all DASes. [05:40] Hmmm, looks like preloading PCJs is causing duplicate queries against PackageUploadBuild [05:41] Not sure how, given uploads should be a list of objects, and builds is a cachedproperty [05:42] Check what is generating the queries/ [05:42] keys.update(map(partial(getattr, owning_object), foreign_keys)) in load_releated [05:42] load_related, sigh [05:43] Oh, wait a minute, why is that calling checkPermission? [05:46] It may be trying to access a secured attribute to find the related IDs [05:51] Bleh, rewritting the PCJ query just pushes the calls to my LFA preloading [05:56] And changing the LFA preloading makes the query count jump to 178 [05:56] Back down to 154, but this is terrible :-( [06:04] Bleh, and that just pushes it down to 'for item in uploads' :-( [06:05] What are you doing? [06:06] StevenK: How's it pushing it down? It should be cached [06:16] wgrant: I'm trying to avoid the stacks of queries that the permission check is causing [06:17] What I mean by pushing down is further along in the method -- it was inside loadPackageCopyJobs, and then in the bulk loading for LFAs for changesfile, and now it's at the end [06:21] StevenK: For the same objects? [06:21] Yeah [06:22] How? [06:22] uploads = list(self.batchnav.currentBatch()) [06:22] They must be cached [06:22] upload_ids = [upload.id for upload in uploads] [06:22] If checkPermission succeeds once, it will be cached [06:22] So I don't see how you're pushing them back [06:22] wgrant: I can upload an OOPS if you wish [06:23] How did you do the permission preloading? [06:23] I haven't [06:23] How'd you move it, then? [06:24] By avoiding load_related() and using upload_ids [06:24] Why? [06:24] Don't you want to check permissions? [06:25] Have you read the PU permission check? It's hideous [06:25] Sure [06:25] But that means you might want to prepopulate the permissions another way [06:25] Not just somehow attempt to work around ever accessing an attribute of the protected object [06:26] In the end you are going to have to access a PackageUpload [06:26] So there's no point introducing ugly hacks to attempt to avoid checking permissions [06:26] wgrant: I'm curious why the upload_ids = [upload.id for upload in uploads] doesn't cause them [06:27] StevenK: id is presumably zope.Public [06:27] Ah [06:28] Right, since we have the SPRs for each PU preloaded [06:30] wgrant: load_related() avoidance hacks removed [08:17] StevenK: You're fixing bug #835645, aren't you? [08:17] <_mup_> Bug #835645: DistroSeries:+queue timeout paginating < https://launchpad.net/bugs/835645 > [08:20] wgrant: Hopefully [08:45] aloha === yofel_ is now known as yofel === _mup__ is now known as _mup_ === apw` is now known as apw === StevenK_ is now known as StevenK === 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 === Ursinha is now known as Ursinha-afk === gary_poster|away is now known as gary_poster === Ursinha-afk is now known as Ursinha === deryck is now known as deryck[lunch] === deryck[lunch] is now known as deryck [19:53] gary_poster: btw [19:53] gary_poster: I know you guys are not focused on LP any more, but I've added some more stuff to testr that may be of interest :) - [19:53] lifeless, hiya. :-) [19:53] gary_poster: hiya :) [19:54] https://testrepository.readthedocs.org/en/latest/MANUAL.html#remote-or-isolated-test-environments and [19:54] https://testrepository.readthedocs.org/en/latest/MANUAL.html#automated-test-isolation-bisection [19:55] lifeless, so this could be used to build test runs across machines the way statik talked about way back when, yeah? among many other things. cool! [19:56] nice lifeless. I'm guessing you are using testr in anger, then :-) [19:57] gary_poster: well I have been for ages; I got some employer sponsored time to step it up a level though. [19:58] gary_poster: openstack's nova project now uses testr as its official test runner [19:58] lifeless, awesome! great. thanks for highlighting the changes--I'll keep 'em in my back pocket [19:58] gary_poster: and yeah, multiple machines would be easier with this I think. Aaron's wrapper should become simpler too. [19:58] cool [19:59] gary_poster: the isolation analyzer is my favourite bling right now though. I think it would have made your analysis work a lot easier. [19:59] ah yes, this is what I was going to do in Go and lost interest in :-) [20:00] Heh :). [20:19] jcsackett, you about? [20:24] sinzui: would you mind looking at https://bugs.launchpad.net/launchpad/+bug/1092643 if you have time please. [20:24] <_mup_> Bug #1092643: Release Management and Downstream Tracking < https://launchpad.net/bugs/1092643 > [20:33] done [20:36] sinzui: cheers [21:46] jcsackett: if you like mashups http://bootiemashup.com/blog/ [21:47] czajkowski: cool! [22:59] wgrant, I revised the branch to convert the 400 to a 404 https://code.launchpad.net/~sinzui/launchpad/path-info-bad-request/+merge/140698