| === slank is now known as slank_away | ||
| 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? | 02:52 |
|---|---|---|
| wgrant | StevenK: What's not working? | 03:23 |
| 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:24 |
| 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:25 |
| 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:26 |
| StevenK | wgrant: What's more concerning is I can't even see the preloading query in the OOPS | 03:28 |
| 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:33 |
| 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:37 |
| 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:38 |
| 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:39 |
| 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:40 |
| wgrant | Ah, yeah, normal users can't create them | 03:41 |
| cjohnston | do you two have admin access? | 03:41 |
| wgrant | No | 03:41 |
| cjohnston | I can't test line 46 then https://code.launchpad.net/~chrisjohnston/launchpad/181725/+merge/140653 | 03:42 |
| 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:43 |
| 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:44 |
| StevenK | Heh, I don't track how many commits I do | 03:45 |
| cjohnston | StevenK: your kind-of on the LP team... I'm community :-P | 03:47 |
| StevenK | So it appears my attempts at preloading to drop the query count don't really help. | 03:51 |
| StevenK | I take that back. Preloading changesfile drops from 237 to 213. | 03:53 |
| * StevenK stabs zope.tal | 03:59 | |
| 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:00 |
| wgrant | StevenK: I'd expect a packagediff existence check, then a packagediff retrieval query, then LFA and LFC queries for each diff | 04:10 |
| StevenK | Yeah, I'm trying to staple packagediff into CPU so I can stop those | 04:11 |
| 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:13 |
| 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 | 04:14 |
| StevenK | wgrant: 237 down to 186. | 05:04 |
| wgrant | Getting there | 05:06 |
| StevenK | Now 167 | 05:07 |
| StevenK | (214-186: Preloading PUBs, BPBs and binary SPRs, 186-167: Preloading all SPNs for source and binary SPRs) | 05:08 |
| StevenK | 153, preloading all DASes. | 05:11 |
| StevenK | Hmmm, looks like preloading PCJs is causing duplicate queries against PackageUploadBuild | 05:40 |
| StevenK | Not sure how, given uploads should be a list of objects, and builds is a cachedproperty | 05:41 |
| 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:42 |
| StevenK | Oh, wait a minute, why is that calling checkPermission? | 05:43 |
| wgrant | It may be trying to access a secured attribute to find the related IDs | 05:46 |
| StevenK | Bleh, rewritting the PCJ query just pushes the calls to my LFA preloading | 05:51 |
| StevenK | And changing the LFA preloading makes the query count jump to 178 | 05:56 |
| StevenK | Back down to 154, but this is terrible :-( | 05:56 |
| StevenK | Bleh, and that just pushes it down to 'for item in uploads' :-( | 06:04 |
| wgrant | What are you doing? | 06:05 |
| wgrant | StevenK: How's it pushing it down? It should be cached | 06:06 |
| StevenK | wgrant: I'm trying to avoid the stacks of queries that the permission check is causing | 06:16 |
| 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:17 |
| wgrant | StevenK: For the same objects? | 06:21 |
| StevenK | Yeah | 06:21 |
| 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:22 |
| wgrant | How did you do the permission preloading? | 06:23 |
| StevenK | I haven't | 06:23 |
| wgrant | How'd you move it, then? | 06:23 |
| StevenK | By avoiding load_related() and using upload_ids | 06:24 |
| wgrant | Why? | 06:24 |
| wgrant | Don't you want to check permissions? | 06:24 |
| 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:25 |
| 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:26 |
| wgrant | StevenK: id is presumably zope.Public | 06:27 |
| StevenK | Ah | 06:27 |
| StevenK | Right, since we have the SPRs for each PU preloaded | 06:28 |
| StevenK | wgrant: load_related() avoidance hacks removed | 06:30 |
| 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:17 |
| StevenK | wgrant: Hopefully | 08:20 |
| czajkowski | aloha | 08:45 |
| === 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 | ||
| 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:53 |
| 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:54 |
| 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:55 |
| gary_poster | nice lifeless. I'm guessing you are using testr in anger, then :-) | 19:56 |
| lifeless | gary_poster: well I have been for ages; I got some employer sponsored time to step it up a level though. | 19:57 |
| 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:58 |
| 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 :-) | 19:59 |
| lifeless | Heh :). | 20:00 |
| sinzui | jcsackett, you about? | 20:19 |
| 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:24 |
| sinzui | done | 20:33 |
| czajkowski | sinzui: cheers | 20:36 |
| czajkowski | jcsackett: if you like mashups http://bootiemashup.com/blog/ | 21:46 |
| jcsackett | czajkowski: cool! | 21:47 |
| 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 | 22:59 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!