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