[00:01] changes to shipit itself? via PQM I believe [00:01] I mean reviewer-wise. [00:01] Since it's ISD. [00:02] dunno [00:02] is there even a project/ [00:02] There is. [00:02] That's where the branch lives. [00:02] wgrant: last time I changed it I just proposed a merge in Launchpad and then pinged somebody in ISD (Anthony Lenton I believe) about it. [00:02] wgrant: He reviewed and took care of landing my changes. [00:02] :( [00:02] Thanks. [00:02] lifeless: I'd be interested in disagreeing with you about that some time. [00:03] lifeless: but not now. [00:10] jml: about what? [ambiguous that] [00:10] lifeless: moving devscripts out of tree [00:10] good night! [00:11] lifeless: Also, you could export id in the interface rather than using rSP. [00:11] jml: Night [00:11] jml: gnight [00:11] Night jml. [00:12] wgrant: err, the ids are exported [00:12] wgrant: they get wrapped, because thats what security proxies do [00:12] then sqlvalues goes boom [00:12] lifeless: Huh? ints are not proxied. [00:12] If it's on the interface it should be fine :/ [00:12] lists of ints are [00:13] Oh, right, all_distro_archive_ids. [00:21] oh man [00:22] launchpad_dev=# SELECT COUNT(*) FROM BugTask JOIN Bug ON BugTask.bug = Bug.id WHERE Bug.id = BugTask.bug AND ((BugTask.status = 10) OR (BugTask.status = 15) OR (BugTask.status = 20) OR (BugTask.status = 21) OR (BugTask.status = 22) OR (BugTask.status = 25)) AND BugTask.milestone = 14 AND Bug.duplicateof is NULL; [00:22] 2 [00:22] launchpad_dev=# SELECT COUNT(distinct bugtask.bug) FROM BugTask JOIN Bug ON BugTask.bug = Bug.id WHERE Bug.id = BugTask.bug AND ((BugTask.status = 10) OR (BugTask.status = 15) OR (BugTask.status = 20) OR (BugTask.status = 21) OR (BugTask.status = 22) OR (BugTask.status = 25)) AND BugTask.milestone = 14 AND Bug.duplicateof is NULL; [00:22] 1 [00:22] our production milestone bug counts are a little inflated [00:23] give you one guess why [00:24] I'm going to go beat my head against a wall for a few minutes [00:26] Hmm. [00:26] librariangc creates dozens of cursors. [00:26] Can anyone see a good reason to keep that? [00:27] I'd check with stub [00:27] I can see no benefit over passing a single one around, instead of the connection that it currently passes. [00:27] k [00:28] anyone remember where the storm expr for Distinct is [00:28] .config(distinct=True) isn't good enough? [00:28] Ah, you want that second query? [00:28] well [00:28] I'm not going to fix the current for loop [00:29] but countBugs also has the same defect [00:29] it just isn't showing it in the current code path because its constrained to only consider series tasks [00:29] the reason conjoined masters mess things up is because they have /two/ tasks [00:29] rather than rendering as two tasks but actually having one. [00:29] Right. [00:38] sigh, we definitely need two queries. [00:38] mmm [00:38] unless, I group by - bingo [00:39] mmm [00:39] no [00:39] our compiler just isn't good enough, and I think we'll get enough benefit for now with two queries [01:10] right, thats another second or so off of bug searching in ubuntu [01:10] + more accurate counts [01:10] Excellent. [01:10] [should be marginally cheaper too] [01:10] we have laze evaluation of milestones too [01:10] going to look at that in a minute [01:14] I can has review number two? https://code.launchpad.net/~lifeless/launchpad/bug-717394/+merge/50541 [01:16] poolie: hi, I will be in syd evening of the 4th,5th and through early avo 6th. [01:16] lifeless: \o/ [01:17] hi lifeless [01:17] great! i'll be here too [01:17] lifeless: milestones are targets now? [01:18] wgrant: for bug search, yes [01:18] :( [01:18] 98 - group_on + (Count(BugTask.bugID),), [], None, params).result_set [01:18] 99 + group_on + (SQL("COUNT(Distinct BugTask.bug)"),), [], None, params).result_set [01:18] it seems like a storm bug if that's any faster [01:18] but it is? [01:19] poolie: how would it be a storm bug? [01:19] wgrant: I can take that out if it doesn't make sense [01:20] does it generate different sql? only becaues of the 'distinct' constraint? [01:20] lifeless: It doesn't seem to be used and doesn't comply with the definition of 'bug target' everywhere else. [01:20] wgrant: it just made sense at the time; I guess rather than setTarget a better name would be 'constrainBy' [01:20] Right. [01:20] wgrant: its used in browser/bugtarget.py [01:21] poolie: yes, different sql [01:21] lifeless, i'm just curious what it does differently [01:21] SELECT COUNT(*) FROM BugTask -> SELECT COUNT(distinct bugtask.bug) [01:21] erm [01:21] SELECT COUNT(bugtask.bug) FROM BugTask -> SELECT COUNT(distinct bugtask.bug) [01:21] ^ thats the change [01:22] and that's faster in psql? [01:23] slightly smaller intermediary table [01:23] the big win is using the countBugs function rather than doing a loop in python [01:23] sure [01:23] there's no way to say 'distinct' in storm? [01:23] doing a distinct forces a sort [01:23] but so does grouping [01:24] so in this case it just lets it return less data internally before the aggregation [01:24] poolie: you can define an Expr for it [01:24] but it likes to put () around things [01:24] and COUNT((Distinct BugTask.bug)) isn't valid SQL [01:24] s/it/storm/ [01:25] Storm will do distinct, but not in a group by. [01:25] Oh. [01:26] I misread. [01:26] But still, no way to do that in Storm right now. [01:26] i wonder if lp should ban branch renames until more of the bugs related to it are fixed [01:26] eg https://answers.launchpad.net/launchpad/+question/146165 [01:27] Is there more than one bug? [01:27] renaming branches that are stacked upon causes troubel [01:27] That is the only real bug that I know around renaming. [01:27] renaming branches causes confusing errors in a bunch of cases [01:30] wgrant: IIRC recipes decided to not link to branches [01:30] wgrant: because modelling recipes was too hard [01:30] or something [01:31] lifeless: False. [01:31] See SourcePackageRecipeDataInstruction.branch [01:32] I think we need a SourcePackageRecipeDataInstructionPartToken, or something like that. [01:32] wgrant: interesting [01:32] wgrant: so why did that guys branch start barfing? or is it a stacking issue [01:32] lifeless: Stacking. [01:32] So, I am all for banning renames of stacked-on branches. [01:32] Because it causes a lot of trouble. [01:33] I think thats appropriate [01:33] but not a desirable constraint per se [01:33] Sure. [01:33] But it *will* break things. [01:33] So we might as well ban it until it's fixed. [01:33] +1 [01:35] * spiv blinks. Isn't "product" a deprecated term? [01:35] Yes. [01:35] Is this on +newrecipe? [01:35] wgrant: no, on clicking the ajaxy bug task affects to reassign something from bzr to bzr-loom [01:36] Ah. [01:36] Thanks; I'll file/find a bug. [01:36] A new one was introduced recently (bug #714536) [01:36] <_mup_> Bug #714536: Branch:+new-recipe related branches listing has formatting issues < https://launchpad.net/bugs/714536 > [01:36] I don't know a bug for yours. [01:39] spiv: yes [01:39] spiv: there is a bug already [01:39] spiv: bug 714534 [01:40] <_mup_> Bug #714534: Branch:+new-recipe refers to "product series" < https://launchpad.net/bugs/714534 > [01:42] Just filed https://bugs.launchpad.net/launchpad/+bug/722412 [01:42] <_mup_> Bug #722412: Changing project of bug task prompts to change/search "product" < https://launchpad.net/bugs/722412 > [01:42] thumper: hmm, that bug to my eyes doesn't seem obviously the same [01:42] spiv: ah... differrent one [01:42] thumper: unless it is meant as a global s/product/project/ bug? [01:42] spiv, no it wasn't [01:43] spiv: just my misunderstanding of your bug [01:43] lifeless: https://code.launchpad.net/~wgrant/launchpad/use-script-isolation-arg/+merge/50543 [01:44] thumper: Heh, I'm guessing you've been working on recipes a lot lately, because I didn't mention them at all :) [01:44] spiv: perhaps it was wgrant mentioning it in passing interleaved with your messages :) [01:46] thumper: he also guessed recipes were involved... I'm detecting a pattern! [01:46] I fixed the bug page as part of some recipe work [01:46] but I didn't notice the product term used [01:49] * thumper is a little confused [01:50] lifeless: are you familiar with the DocTestMatches matcher? [01:52] lifeless: nm [01:53] yes [01:53] I wrotes it [01:53] the whole point of using the DocTestMatches was to have it normalize the whitespace. A little frustrating that I have to pass in the flag [01:54] when I wrote it I wasn't sure what sensible defaults would be [01:54] if we were to put the flag on by default [01:54] I think we'd need a way to turn it off too [01:54] but we could change the default, or you could use functools.partial to make a version that has the flag supplied for all your tests [01:55] hmm... [01:56] lifeless: Thanks. [01:57] thumper: idioms that work in functional environments will work well with matches [01:57] * thumper off for school run [01:57] thumper: its a very similar environment [02:00] ok, that was too easy. [02:03] if I put a method 'getPillar' on I*Series and I*[things that also have series variants] [02:03] what would you expect it to do? [02:03] Isn't there a .parent or similar to do that? [02:04] ah, but IProduct.getPillar() -> self [02:04] possibly I want IPillar(thing) [02:04] do we have that already [02:04] ? [02:04] We have IRootContext or similar, which does that. [02:05] But it's probably not a good idea to reuse it. [02:05] another review plox https://code.launchpad.net/~lifeless/launchpad/bug-717394-2/+merge/50545 [02:05] o/ spiv [02:06] * spiv waves [02:11] wgrant: if you look at my new branch you can see it would be rather more pithy and clear [02:11] wgrant: I don't know how often we do this [02:15] * lifeless headdesks [02:15] lib/lp/registry/model/distribution.py [02:15] lifeless: Sure, I would support an IPillar. Take stuff out IRootContext. [02:15] line 724 - __getitem__ [02:15] Wow. [02:16] OTOH it does save queries. [02:16] Hm, except not. [02:16] But Distribution.series is cached, so yes, it does save queries. [02:16] Slightly. [02:16] huwshimi, does https://bugs.launchpad.net/launchpad/+bug/722419 ring any bells? [02:16] <_mup_> Bug #722419: bug title edit buttons go under portlets, can't be clicked < https://launchpad.net/bugs/722419 > [02:17] it triggers the query I'm seeing [02:17] poolie: I can reproduce it if that's what you mean? [02:17] whether it saves queries or not is a little dicy at this point [02:18] I'm not sure thats a regression, its been like that since ajaxification [02:20] poolie: Oh actually that's a dupe [02:21] huwshimi, i guess i was politely asking "i wonder if you broke it" :-) [02:21] lifeless: I like Distribution._sort_key [02:21] not really a big deal [02:22] poolie: haha, ok. No I didn't. It's actually been like that for ages. It has to do with how big your browser window is [02:24] yes [02:24] wgrant: facedesk [02:24] poolie: Have you changed your res or browser size recently? [02:24] poolie: Like in the last week :P [02:24] wgrant: you'll love 722421 [02:25] bug 722421 that is [02:25] <_mup_> Bug #722421: lib/lp/bugs/model/bug.py _known_viewers causes per-row lookups on public bugs in bug searches. < https://launchpad.net/bugs/722421 > [02:26] It's also wrong. [02:27] argh [02:27] wgrant: no, its not [02:27] bac broke the optimisaiton [02:27] lifeless: Pillar owners cannot always see bugs... [02:27] either way [02:28] whether its correct to do so or not [02:28] the defect is real [02:28] Also some of the manipulations of that cached property seem to be a little screwed. [02:28] # The bugtask is unassigned, so clear the _known_viewer cached [02:28] # property for the bug. [02:28] get_property_cache(self.bug)._known_viewers = set() [02:28] rev 12156.8.20 [02:28] the clearing is conservative [02:28] That's not clearing it. [02:28] That's *emptying* it. [02:28] indeed [02:28] so [02:29] the branch to set assignee as a viewer as broken [02:29] bug 702429 [02:29] <_mup_> Bug #702429: Pillar owners and private bug visibility < https://launchpad.net/bugs/702429 > [02:30] wgrant: however the contract for it is that it can be an empty set and is added to [02:30] wgrant: so del() would be equally glitchy [02:31] Yes, but it would at least crash instead of being wrong. [02:31] wgrant: its not wrong [02:31] wgrant: if a user is not in the set the rest of the 'can see' code triggers and it will repopulate [02:31] Ah, I see. [02:32] this is fallout from folk still learning about how eager loading and late evaluation interact [02:32] another 75 queries a page removed [02:33] Assuming each pillar is distinct. [02:33] Which will happen approximately never. [02:34] what was also buggy [02:34] was the missing TP lookup [02:35] wgrant: uhm, no, it requeries the *bugtasks* per bug [02:35] wgrant: so you don't need distint pillars, you just need multiple bugs [02:35] Oh. [02:35] Nice. [02:35] I didn't actually read the bug past the summary, FWIW. [02:36] I am looking to see if there is a sensible way to put tracebacks in the OOPS format. [02:36] I suspect not. [02:36] there isn't [02:37] I'm kicking off a multi-team requirements list for oops tools [02:37] give it a couple of days and I should have their needs and be able to write it up for public consumption [02:37] Great. [02:37] For now I guess I'll just write invalid OOPSes locally... [02:40] poolie: that bug is a dupe of #692291 which was reported in December. I' [02:40] <_mup_> Bug #692291: Impossible to modify the title of the report

< https://launchpad.net/bugs/692291 > [02:41] poolie: I'll mark yours as a dupe. Feel free to elevate the existing bug if you feel the need [02:52] huwshimi, i don't think i changed my screen resolution recently [02:52] hm, i did tweak fontconfig to use the ubuntu mono beta font [02:52] it's possible that changed the layout [02:55] my bug title was better :) [02:55] where do people get these things [02:56] poolie: Yes, your description was probably better too [02:57] poolie: welcome to the problem of using a combination of relative and fixed sizing :) [02:59] lifeless: It looks like the problematic change was actually a little earlier... 12126.4.1 [02:59] I don't know why 12156.8.20 seems to have that change too... [02:59] poolie: Although the mono font doesn't look like it should have affected that [02:59] it was meant to be reverted according to the bug bac filed [02:59] I wonder if someone remerged it in accidentally [03:00] THe diff in the MP shows it being changed, not restored or removed. [03:00] Odd. [03:00] maybe i just noticed it today [03:03] Oh yeah, github's branch browser is nice. We should totally have one that doesn't suck. [03:06] lifeless: Hmm. The SQL statement value substitution has broken duplicate statement detection completely. [03:06] That's a bit sad. [03:06] I think. [03:06] Or maybe this view just does lots of raw SQL. [03:08] wgrant: python strings send to python-pgsql are u"foo" - this threw lpoops off for a few days [03:08] but it should be fixed now [03:08] whats ths oops [03:09] ah [03:09] milestones also trigger lazy evaluation on bug searches [03:13] Huh. [03:14] In OOPS-1878QS4 I see lots of queries for people who are message owners... but those comments came out of memcache. [03:14] the bind variables stuff captures at our layer, not what the serialised form of real sql would be [03:14] if yo usee what I mean [03:15] wgrant: makes sense [03:15] Why? [03:15] wgrant: memcache is incompatible with eager loading [03:15] Sure. [03:15] But this is a separate query for each message. [03:15] hmm [03:15] yes [03:15] Which means something is accessing message.owner. [03:15] right [03:15] Something other than the message view. [03:15] permission checks [03:15] On a message? [03:15] Messages have no security. [03:15] read get_visible_messages [03:16] wut [03:16] * wgrant needs a sharper desk. [03:16] now you see why I was like \o/ when I got shit working [03:18] Are you burning that, or should I? [03:18] wgrant: I took it out of the key path for performance [03:19] wgrant: by ignoring it for range query purposes [03:19] wgrant: we need to push it down to the db, delete things that we don't want, etc. [03:19] wgrant: are you seeing VPC calls ? [03:20] wgrant: if so, the getMessagesForView eager loading may be incorrect [03:21] lifeless: There are also VPC calls, which I was about to look into. [03:21] There are also another 60 person queries unaccounted for, even for anonymous requests. [03:21] I'm trying to track those down. [03:22] cool [04:26] another review plox https://code.launchpad.net/~lifeless/launchpad/bug-717394-4/+merge/50548 [04:44] * StevenK grumbles at tal [04:45] It works for one case, but not the other? :-( [04:49] thumper: Still around? [04:55] hmm [04:55] 5 [04:55] branches today [04:56] Not bad. [04:56] Should I do more ? [04:56] Yes. [04:56] Always. [04:56] it is nice to be doing stuff [05:00] lifeless: Can haz a little TAL help? [05:00] sure [05:01] how do you like it ? [05:01] lifeless: I can pastebin a diff and a traceback? [05:01] sure [05:02] wgrant: ahha, I think I know why the subscriptions are needed to do bug pages [05:02] File "/home/robertc/launchpad/lp-branches/working/lib/canonical/launchpad/webapp/menu.py", line 262, in _buildLink [05:02] linkdata = method() [05:02] File "/home/robertc/launchpad/lp-branches/working/lib/lp/bugs/browser/structuralsubscription.py", line 338, in subscribe [05:03] if sst.userHasBugSubscriptions(self.user): [05:03] lifeless: Just had a thought, let me try something [05:03] Hah. [05:11] lifeless: Right, I have it working, but not how I want [05:12] garh fail [05:13] the vocabulary to change bug targets [05:13] type 'launchpad' in there [05:13] lifeless: http://pastebin.ubuntu.com/569920/ [05:13] lifeless: Looking at the page in my browser, I get: "Built by recipe generic-string548131 for Person-name548129" which is brilliant, but I'd like the recipe to be a link, and it isn't. [05:13] StevenK: and what do you want it to do? [05:14] you don't want tal:replace [05:14] lifeless: And given this is first time ever touching tal, I suspect I'm missing something fundamental [05:14] replace replaces a tag by something [05:14] e.g. structure, a string or a context [05:14] you want to set the href [05:15] e.g. [05:15] tal:attributes="href context/bug/duplicateof/fmt:url">bug [05:15] #42 [05:15] bah [05:15] before that [05:15] Right [05:18] lifeless: Excellent, thanks [05:18] cool [05:18] wgrant: https://bugs.launchpad.net/launchpad/+bug/722455 may entertain you, amidst more headbanging [05:18] <_mup_> Bug #722455: bug task target selector widget cannot be used to select 'launchpad' < https://launchpad.net/bugs/722455 > [05:19] lifeless: Oh, that's even more awesome than last time I tried. [05:20] lifeless: It does the same thing for 'launchpad-project' too [05:20] win [05:20] though launchpad-project can't have bugs, it would be nice to list all the elements of it there [05:21] wgrant: Did you see https://bugs.launchpad.net/launchpad/+bug/722344 ? [05:21] <_mup_> Bug #722344: PPA traversal assumes distribution is ubuntu < https://launchpad.net/bugs/722344 > [05:21] StevenK: Yes. [05:21] Let's design URLs without the distribution in them! It's going to be awesome! [05:21] And no wails of dispair. Awesome. [05:22] ok [05:22] why does bugtask need the main archive and archive admin person as json [05:23] Hah. [05:23] however [05:23] I've now audited distro bug search [05:23] I didn't realise we pracached the context as JSON. [05:24] that might explain the what of it. Tho not the why [05:32] lifeless: http://people.canonical.com/~stevenk/built-by-recipe.png [05:33] StevenK: that looks pretty nice, but why is the word 'for' in ?blue? [05:34] lifeless: The whole thing is one link to the recipe [05:34] I wouldn't link the 'for' or person name then [05:34] Right [05:34] I think folk would expect the person to be a link to the person, if anything [05:35] Okay, fixed [05:35] lifeless: Just wanted your thoughts before I put together an MP and beg for a UI review [05:36] StevenK: de nada [05:36] (My first, surprisingly) [05:36] UI review? Surely you jest. [05:36] wgrant: I have your manager on the phone, he wants a word. [05:37] :-P [05:37] TALES, please stop swallowing every exception under the sun with LocationError... [05:38] wgrant: Ah yes, I've been hitting that today too [05:38]