[01:11] StevenK: Hm, you didn't move the two constants? [01:11] AFAICS you just renamed them. [01:11] PRIVATE_BUG_TYPES etc. [01:11] I was supposed to move them? [01:12] I said they should be alongside the enum. [01:12] Perhaps I was unclear. [01:12] But there's nothing bug-specific about them. [01:16] wgrant: Sorry, I've moved them to registry.enums [01:16] Thanks. [01:20] sinzui, That page seems to scroll really slowly in my Firefox. [01:21] cody-somerville: For which project? [01:21] bugsy [01:22] Approximately how many rows does it have? [01:23] a lot. [01:23] Ubuntu has nearly 20000, which is a lot. [01:23] Launchpad has a few hundred, which is a lot. [01:23] What is a lot? [01:24] probably less than 100 [01:25] Ah [01:25] Not a lot :) [01:29] wgrant: I guess I should kill IBug.set{Private,SecurityRelated} and related gubbins? [01:29] StevenK: Something like them is still necessary for the API. [01:29] (and +secrecy) [01:30] Rargh, setPrivacyAndSecurityRelated is still exported. === matsubara is now known as matsubara-afk [02:34] StevenK: you ocr? [02:49] StevenK: Here, I made three code reviews, just for you :) [02:51] and one from me [02:51] https://code.launchpad.net/~wallyworld/launchpad/tri-state-sharee-picker-959784/+merge/98556 [02:51] it's mainly javascript too, and i know StevenK loves javascript :-) [02:52] and if he does them soon, we can make the next buildbot run :-) [02:54] I was reviewing my lunch. [02:54] nom [02:54] wgrant: Three? I can only see two. [02:55] It would help if I submitted the MP for the last one. [02:56] Haha [03:02] wgrant: All three done. [03:02] wallyworld_: You're up now. [03:02] StevenK: Thanks. [03:07] wallyworld_: Three comments, the first two are not optional, the last one is. [03:14] StevenK: thanks [03:51] wallyworld: If you hold down the "Propose merge" button it brings up 3 or 4 spinners and then errors horribly [03:51] StevenK: so don't hold the button down :-P [03:52] lol [03:53] StevenK, wallyworld: Clearly we need to have a notice that says "do not press more than once". I think that's how the banks handle it. [03:54] huwshimi: yes, or something like that. disable on click even [03:54] No, I think we need do it on mouseUp [03:54] yes, you are right [03:54] * huwshimi thinks wallyworld missed the joke [03:54] I didn't think you got mulitple mouseDown events if you held the button down [03:55] me wither [03:55] either [03:55] stub: wgrant: have the two of you spoken about not using FK's in the FACT tables ? [03:56] No, but indeed we should. [03:56] stub: So I heard you like foreign keys. [03:56] ("Yo dawg") [03:56] I just imagined wgrant say it with that added. [03:57] wgrant: I shirley dooly [03:58] stub: I'm not sure they're valuable on fact tables. [03:58] And in some cases they're not possible. [03:58] eg. denormed arrays [03:58] wgrant: Looks like they will be in 9.2 or 9.3 :) [03:58] But sure, if PG doesn't support it that is a good reason to not use that case. [03:58] Given that they're maintained by tested triggers from FKed columns, the only purpose they serve seems to be to cause slow writes :) [03:58] Oh really? [03:58] Hmm [03:59] Oh, I see a patch. Nice. [03:59] I'm fine with not having them if the table is driven by triggers. [04:00] It is a harder decision to make if writes are distributed over more code, as we are more likely to screw up. [04:00] Right. For tables that are fairly simple, written only by triggers, and easy to regenerate quickly if something does break horribly, I think they're not worth it. [04:00] If app code is touching them, agreed. [04:01] So are the fact tables going to reference libraryfilealias (for, say, patches) or person? [04:02] They reference Person, but not LFA. [04:02] Which is slightly interesting. [04:02] Will need some special well commented code in person merge then [04:02] But the triggers handle the person merge case fine. [04:02] Ok. [04:02] oh, of course. [04:03] is all fine [04:04] That array FKs thing is really nice, though. [04:05] Might actually make them practical for normal use. === wallyworld_ is now known as wallyworld [05:11] poolie: hi [05:12] wgrant: small arrays only probably, don't want them TOASTed (to be better than a separate table.. perhaps) [05:15] lifeless: TOAST isn't too bad in some situations. [05:15] its fine as long as you're not using an ORM that requests every field always [05:15] Ahem [05:15] ok [05:15] so thats hyperbole [05:15] but not all that far off [05:15] (fortunately the ORM isn't going near my array table :)) [05:16] Also, isn't the TOAST threshold like 8KiB? [05:16] That's one large array. [05:16] nah [05:16] that would be 2 pages [05:17] lifeless, hi [05:17] 2K I think [05:17] Pages are 8KiB. [05:17] does lp actually dislike docstrings on tests, or does it just tend to not have them? [05:18] bit of both [05:18] they aren't very useful, and the rules for docstrings are rather awkward for comments on many tests [05:18] also they tend to be redundant with well chosen test names [05:18] I prefer a comment at the top of a test, but I may forget to add one. [05:19] wgrant: I have no idea why I thought 4KiB [05:19] lifeless: Because every other computing definition of "page" is 4KiB. [05:19] yes [05:20] The TOAST code is triggered only when a row value to be stored in a table is wider than TOAST_TUPLE_THRESHOLD bytes (normally 2 kB). The TOAST code will compress and/or move [05:20] http://www.postgresql.org/docs/8.4/static/storage-toast.html [05:20] Ah [05:22] I concede. [05:24] i know unittest used to have stupid behaviour where it would not print the test name if there was a docstring [05:24] but i'm pretty sure that's fixed in lp's test runner [05:24] poolie: I think our testrunner still does. [05:24] Hmm [05:24] Maybe that was fixed. [05:24] Or I'm thinking of another project. [05:24] Oh goody. [05:24] code.launchpad.net is magically faster now. [05:24] And Bugs is slowly speeding up. [05:24] wtf launchpad why are you doing this [05:24] don't get better without us doing anything :/ [05:30] * wgrant breaks the world with HTML5. [05:36] stub: so wtf with bugtag.name [05:36] stub: how was 8.4 working at all ? [05:36] lifeless: I have no idea [05:36] stub: tests failed for you ? [05:37] lifeless: Either somehow that code path was never reached, or there is some odd scoping going on with 8.4 that allowed it to work (maybe another table with the same alias in the outer scope?) [05:37] Yes, picked up with a failed test run. Testing my 9.1 ec2 image. [05:37] That code path should have been reached, though :/ [05:37] I'd seen it before, but thought I'd fixed it already. [05:38] stub: That code is fairly new [05:38] There are still a few other tests failing I want to fix later today. [05:38] And should really just be deleted. === jtv is now known as jtv-afk === almaisan-away is now known as al-maisan === danilo_ is now known as danilos [07:42] salgado's email re: conjoined bugs confuses me, and leads me to fail to discover any actual definition of a conjoined bug. [07:45] _NON_CONJOINED_STATUSES = (BugTaskStatus.WONTFIX, ) is probably what confuses me the most [07:46] stub: That's the bit of the definition that nobody remembers. [07:47] A bugtask conjoinment relationship has two halves: the master and the slave. [07:47] The master task is the development series task: a productseries, distroseries, or sourcepackage task. [07:48] The slave task is the non-series task: a product, distribution or distributionsourcepackage task. [07:48] Conjoinment implicitly occurs when there is a task for the development series. [07:48] The slave is no longer directly editable, instead being mirrored from the master. [07:48] *Except* when the master is marked Won't Fix, in which case the conjoinment relationship is broken, and the slave is separate again. [07:49] It's like somebody tried to devise the most obscure set of rules they could think of. [07:50] Yer. Your definition makes sense. I suspect it would have benefited from rethinking the data model around those rules at the time. [07:50] You'd think. [07:51] well [07:51] the whole problem was one of model leaking [07:51] nominations contribute to the pain too [07:51] skaet would love only-series tasks to be implemented :) [07:52] And most of the rest of the world would not :) [07:52] well [07:52] I don't think thats true [07:52] most of the rest of the world wouldn't ever notice [08:56] good morning [09:06] aloha [10:49] gmb, hi there! are you up for reviewing a fix for me? :) https://code.launchpad.net/~linaro-infrastructure/launchpad/notify-workitems-changes/+merge/98231 [10:58] I probably didn't mean gmb since it looks like he's not online. :) === al-maisan is now known as almaisan-away [11:15] mabac: I'm mobile ATM. Also, it's [11:15] Not [11:15] My review day. [11:16] gmb, sorry, I just thought I'd bug you since the topic listed you as OCR. :) [11:16] mabac: Check http://dev.launchpad.net/ReviewerSchedule for details of who's on today. [11:16] It's out of date. Apologies. [11:16] gmb, thanks! I didn't know about that page === gmb changed the topic of #launchpad-dev to: https:/​/​dev.launchpad.net/​ | On call reviewer: - | Firefighting: - | Critical bugtasks: 4*10 [11:17] Doing that on my phone was hard :/ [11:17] :/ [11:17] gmb: ello ello [11:17] gmb, :) thanks [11:18] Hi czajkowski. [11:18] gmb: fancy a UDS photo walk one of the evenings? [11:18] czajkowski: Definitely! [11:18] hop on the bart and walk around san fran take pics and grab foods? [11:18] plan it in advance and get some numbers? [11:18] and you cna plan a route :) [11:18] know there are a lotta photo folks in ubuntu could be fun [11:18] something different to do [11:19] Haha. My plans often fail, hard. But it seems like a good principle. I'll put something together and we can refine it together; sound cool? [11:19] am gonna update the uds wiki page [11:19] will mail you in a bit [11:19] Cool. [11:19] and then get it posted to uds organisers :) [11:20] * gmb afk to give someone a large amount of money [11:20] if people know in advance then it wont clash with team dinners [11:27] czajkowski: Right; makes perfect sense. [12:31] stub, who's working on the private bugs overhaul you mentioned? [12:32] Curtis' team [12:33] sinzui: ^ [12:34] purple squad working their magic [12:37] cool, thanks stub [12:47] salgado: It's not so much as an overhaul as a complete redesign and rewrite from scratch. [12:47] get_bug_privacy_filter is what you should be using, indeed. [12:47] We'll have to rewrite your query soon, though. [12:48] wgrant, ok, cool. but why rewriting my query is going to be needed? === matsubara-afk is now known as matsubara [12:49] salgado: Because there's a new, faster bug search schema coming soon, and privacy is completely different. [12:50] oh, ok [13:07] I'm waiting for abentley who seems to be ocr in the next time-slot. Anyone know when his day starts? Or even anyone who would like to review this for me? :) https://code.launchpad.net/~linaro-infrastructure/launchpad/notify-workitems-changes/+merge/98231 [13:08] mabac: he should be in soon. [13:08] our stand up is in 30 [13:08] so will have to catch him after that probably [13:08] thanks rick_h === beuno_ is now known as beuno === abentley changed the topic of #launchpad-dev to: https:/​/​dev.launchpad.net/​ | On call reviewer: abentley | Firefighting: - | Critical bugtasks: 4*10 [13:31] abentley, good to see you. I hope you're happy to learn that I'm hoping to add to your work load this early ;) [13:32] abentley, https://code.launchpad.net/~linaro-infrastructure/launchpad/notify-workitems-changes/+merge/98231 is in need of a review [13:32] mabac: On The Phone [13:42] mabac: wow you really want this code reviewed! [13:44] czajkowski, sorry if I'm being a pain. just trying to get this off my plate since I'll be offline the rest of the week. I'll shut up now ;) [13:45] abentley, sure no rush. really. I'm sorry for jumping on you like that [13:59] jcsackett, I am indeed trying to get you branch merged. [14:00] jcsackett, I decided to PQM landed it after getting another false error in ec2 test [14:00] sinzui: got it, so i should not attempt to reland on my end. :-P [14:00] There is now a text conflict though [14:01] sinzui: if you have not pqm-landed it, i should be able to land on my end now. [14:01] or if you're already dealing with it, i'm happy to leave it to you, if you would prefer. [14:01] jcsackett, Was fixing the conflict. let me try for 2 more minutes [14:01] sinzui: sure. [14:06] jcsackett, I am going to run the pillar tests locally. You and Ian collided [14:06] sinzui: ok. [14:07] adeuring: http://pastebin.ubuntu.com/892304/ [14:18] abentley: can't header you any more [14:19] abentley: mumble instead? [14:19] adeuring: Sure. [14:21] jcsackett, your branch is merged. [14:21] sinzui: awesome, thanks. :-) [14:22] glad i sorted out what you were doing before lp-landing on my own. :p [14:24] jcsackett, the conflicts were nasty. merge is an idiot. I reviewed both your branch and Ian's so I just pasted your classes into the conflicting module instead of editing the spaghetti. [14:25] sinzui: that seems very reasonable, and having just updated from devel it all looks right to me. === almaisan-away is now known as al-maisan [14:32] mabac: Your functionality looks okay. [14:32] mabac: You are adding LOC. Do you have permission to do that? [14:33] abentley, I hope this is considered offset by the tech debt payed off by salgado for the work items feature [14:33] abentley, this is a bug fix, not a new feature, btw [14:33] abentley, so my answer is, yes I believe so [14:34] salgado: Is there a blanket exemption for bug fixes? I didn't think there was. [14:35] abentley, still this should have been part of the initial feature, just neglect on my part that it wasn't [14:35] mabac: Okay. [14:36] mabac: Some of the indentation doesn't match our style. 40-41 should be indented four spaces. 42-43 should be indented another four spaces. [14:36] bac: if you get a chance would you mind looking over https://answers.launchpad.net/launchpad/+question/169663 please. [14:37] abentley, maybe there isn't, but I remember someone saying that in the end it's at the reviewer's discretion to decide and requiring contributors to remove unused code before they can fix bugs sounds like a very good way to chase them off [14:38] mabac: at 56, since the arguments don't all fit on one line, they should all be placed on the next line, indented four spaces. [14:39] abentley, ok cool. [14:40] salgado: No, there's no room for reviewers discretion. waivers are granted by flacoste and lifeless. [14:42] abentley: yes, salgado's credit is still good on that one [14:47] flacoste, that's cool, but do you think it make sense to require people to remove unused code in order for their bug fix to be accepted? [14:48] salgado: No, there's no room for reviewers discretion. waivers are granted by flacoste and lifeless. [14:48] mabac: Also 138-142, 148-149, 170-171, 185-190, 191-196, 205-206, 210-213 [14:48] mabac: Is the lint clean? [14:48] abentley: yes, salgado's credit is still good on that one [14:48] <-- abentley has quit (Ping timeout: 264 seconds) [14:54] abentley, apparently not. looks me relying on my emacs mode indentation and pep8 wasn't a good idea. can you tell me how I run lint to check the lp style? [14:55] mabac: I don't know of a linter that check's Launchpad's style, but you should run "make lint" to ensure your code is lint-free. [14:57] abentley, ok sorry I hadn't done that. I get a couple of errors but they are not caused by this particular change. they're still my doing so if I can sneak in fixes for those here that'd be great. [14:58] mabac: Please do. [15:06] adeuring: The problem still exists, but storing job.fail as a local variable avoids it. [15:07] abentley: ah, ok. So you were right that the problem is caused by late loading job. [15:10] abentley, I have pushed changes, I hope it looks better now [15:13] mabac: Thanks. That's better. At 57, 150, 172, 209, the line should be indented four spaces. [15:14] czajkowski: that's interesting. it's also interesting that we've not seen a general problem with key import so i'm not sure what is happening. [15:15] mabac: At 143, 192, 199, I believe the } is indented 4 spaces too far. I.e. it should have the same indentation as the line containing the matching {. [15:16] bac: aye I wonder is it something specifically wrong with that key and why they don't generate a new one rather than wait 6 months. but how and ever [15:17] czajkowski: yes, it is odd [15:17] mabac: Did you investigate salgado's comment "However, if someone edits the workitems using the +workitems page, no notification will be sent because that code doesn't call updateStatus() via lazr.restful and so no event is fired. We might need to have that page itself fire an ObjectModifiedEvent..." [15:17] ? [15:17] czajkowski: i do see his key is crazy old...1996 [15:18] abentley, yes. that's handled by change line 56 [15:19] mabac: great. [15:19] bac: makes no sense we'd surely have others with the same issue [15:22] abentley, I've pushed another revision with the indentation fixes [15:24] mabac: I think your comment in tests, "In production this notification is fired by lazr.restful..." is now stale. [15:25] mabac: Also, you should add a description of each test-- either a docstring or a comment. [15:25] mabac: Do the tests still need transaction.commit? [15:25] abentley, the comment was intended to explain why I fire a notification in the tests but not in the production code. if that's not useful I'll remove it [15:26] abentley, they do. and I've followed an existing example where it's done exactly like that [15:26] abentley, ack on the docstrings [15:26] mabac: But "this notification is fired by lazr.restful" is true, but not complete, now that it's also fired in another location. [15:27] abentley, true. I'll fix it [15:28] mabac: r=me with those changes. [15:29] abentley, thank you very much! [15:29] mabac: np. Sorry for the delay. [15:29] abentley, that was very quick so I'm very happy :) [15:31] mabac: line 57 still looks wrong. [15:32] abentley, sorry about that, I had forgotten to save that file [15:34] abentley, fixed and docstrings added [15:35] mabac: great. Do you need me to land this for you? [15:35] abentley, if you would. thanks! [15:36] abentley, that is, yes please :) [15:37] mabac: okay, will do. [15:37] abentley, thank you [15:39] jcsackett, I sent you an image to add to your mockup directoy [15:42] jcsackett, no change from your work. We are adding a picture based on wallyworld's discovery that the suggested design overwrites permissions or fails to inform about existing permissions [15:42] sinzui: rereading your email now, sorry. :-P [15:43] danilos, mabac, the call is in 20 minutes. are you guys joining? [15:43] sinzui: pushing it up to the directory now. [15:44] thanks. === al-maisan is now known as almaisan-away [15:46] salgado, I am, mabac won't be able to [15:54] mrevell, salgado: hi, I suppose mumble or hangout will do? [15:54] salgado, flacoste, danilos: Mumble is fine. === abentley_ is now known as abentley === almaisan-away is now known as al-maisan [16:00] salgado, flacoste: We're in the Product channel on Mumble [16:07] flacoste, joining us? [16:09] danilos, salgado: https://dev.launchpad.net/Projects/WorkItems/ === al-maisan is now known as almaisan-away === almaisan-away is now known as al-maisan === salgado is now known as salgado-lunch === al-maisan is now known as almaisan-away === salgado-lunch is now known as salgado [17:54] mrevell: sorry, i had a dr appointment, i forgot to tell you [17:55] flacoste, Ah no problem. I'll send notes for the checkpoint later. The actions are for Linaro to produce the milestone team view and test internally, with danhg doing some testing with a broader group of users. danilos also had a question about how we handle groups of work items within an indiviual blueprint, which he posted to the -dev list. [18:05] ok [18:30] gmb: https://wiki.ubuntu.com/UDS-Q/OtherEvents [18:45] czajkowski: Ah, thankee. I've already turned up one or two potential routes. Really depends what we're after (landmarks vs interestingness). [18:46] gmb: factor in a food stop [18:46] czajkowski: Right. And also travel time from t'hotel, which, this being the BART, could be substantial. [18:47] Anyroadup, I'm sure we'll come up with something. [18:47] gmb: 30 mins I think [18:48] czajkowski: Yeah, it's not so much the travel time as the waiting-for-a-train time. [18:48] nods [20:37] flacoste: hi [20:37] hi lifeless [20:38] flacoste: did you see skaets in-line request for escalation on the audit trail bug ? [20:38] yes [20:38] salgado: hi, when is your EOD ? [20:38] flacoste: cool [20:38] lifeless: i replied to her via irc, but i think i'll need to follow-up more [20:38] lifeless: basically, i don't think we have time to do it before the release [20:38] so not sure it's worth escalating at this point [20:39] yeah, I had a private chat too [20:39] the bug wasn't on our management radar [20:39] lifeless, in about 20 minutes, why? [20:39] salgado: I thought I might try and help you with this bug search thing [20:39] salgado: if you would like to talk about it, irc or voice or whatever [20:40] lifeless, sure, mumble, hangout or something else? [20:41] skype or hangout work best for me [20:41] mumbles lack of echo cancellation -> pain [20:41] flacoste: if you need anything from me (that I'm not already doing :P) on that bug, let me know. [20:42] i will [20:42] lifeless, agreed. starting a hangout now. [20:43] wow, unity's alt-tab is reallly hard to use [20:43] just breaking me all the time atm [20:44] lifeless, http://paste.ubuntu.com/894246/ [21:00] lifeless, https://dev.launchpad.net/Projects/WorkItems [22:15] has anyone else seen an error about [22:15] 2012-03-21 06:14:39 UTC FATAL: could not access private key file "server.key": Permission denied [22:15] running current lp? [22:26] poolie: No. Possibly an SSH server key? [22:27] google suggests something about an ssl key [22:27] which would make more sense for postgres [22:27] (i should have mentioned that's where it is) [22:28] probably something local [22:30] Oh, if that's postgres, then yes, it's probably postgres :) [22:53] wallyworld, Surely there is something in this list that irks you: https://bugs.launchpad.net/launchpad/+bugs?field.tag=trivial [22:58] Morning. [23:01] if i grant a team access to a pppa, will they all get spam^Wmailed? [23:01] o/ sinzui [23:01] o/ nigelb [23:01] yes [23:01] Hey poolie! Got a new motorbike yet? :) [23:02] not yet, but i did get the payout for the old one [23:02] i'm not going to get the chance to ride much for the next while so i might wait [23:02] hi poolie [23:02] Oh, nice. [23:02] * huwshimi goes digging to find out who broke the homepage [23:03] lifeless, that 'yes' was to me? [23:03] :/ [23:03] poolie, do you think the emails should go to the team admins? or none at all? [23:04] not sure [23:04] maybe it would be nice to have a tick box for "mail people to tell them they have access", like google docs does for instance [23:04] or, just silently give them access [23:04] presuming they'll find out some other way [23:04] huwshimi: You mean the header being wider than the rest of the page? [23:04] it doesn't seem like a thing where there's a strong reason they _must_ be told [23:04] wgrant: Yeah... [23:05] huwshimi: sinzui removed locationless a while ago. [23:05] That's what changed it. [23:05] revno: 14867 [merge] [23:05] timestamp: Sat 2012-02-25 07:26:36 +0000 [23:05] wgrant: Oh, so this has been live for a while!? [23:05] message: [23:05] [r=bac][bug=432889,435832,580240,938889] Update error pages to look [23:05] and read like other Lp pages; locationless is gone. [23:06] poolie, there is a similar bug request for mailing lists. It would be nice if Lp could show you new things on your pages that you got access to when you arrive [23:06] indeed, i guess that connects to a more generally nuanced view of notification [23:06] so "tell them now" would perhaps [23:06] mean [23:06] "it's important they know now", which would push it up to being sent by mail [23:07] or, maybe there should just be a per-user "tell me when i get a new ppa" flag [23:07] s/tell/mail [23:07] anyhow, generic notifications ++ [23:07] I pioneered a mechanism to show notifications from non-browser code last week. It is possible to store them then unqueue them when the user arrives [23:07] StevenK: Should 933759 be in QA-Landing? [23:08] poolie: yes [23:09] wgrant: So I'd like to add information_type to branch's model code and then use that branch to close the bug. [23:10] StevenK: Mmm. [23:10] StevenK: It's not useful yet, and won't be for a while. [23:11] wgrant: Or we can just close it saying the column exists and other bugs deal with switching to it. [23:11] Yes. [23:11] That was my intent. [23:11] Doit [23:11] wgrant: with the missing sorting in getPillarShareeData, why wouldn't I just add .order_by("person_sort_key(Person.displayname, Person.name)") to the findGranteePermissionsByPolicy() query? [23:12] you mentioned not to do that but i think it's the way to go [23:12] that's effectively how it was before i split the query [23:13] wallyworld: You shouldn't be loading people in that query. [23:13] Given how slow loading people is. [23:13] I guess you could do it, given that it's batched now. [23:13] yes i think so [23:13] it's only a few records eg 75 [23:13] we can optimise later if really needed [23:14] That's still around 40ms :/ [23:14] that's negligable [23:15] mm [23:15] I think we need to recalibrate our speed expectations :) [23:16] 40ms is actually quite slow [23:16] Yes. [23:16] That's nearly 10% of the average render time of Product:+index [23:16] Which is still slow. [23:16] in the context of a round trip it's ok surely [23:16] and the rest of lp loads people in queries as needed [23:17] Sure, but the rest of LP is fucking terrible. [23:17] This stuff doesn't have to be :) [23:17] and we're talking max batch_size records [23:17] let's just get sorting fixed first and then optimise if needed [23:17] i sure wouldn't notice if a page rendered 40ms slower [23:18] It's a 40ms delay for a non-significant query. [23:18] On its own it's not terrible. [23:18] so? in the content of the overall page render time, it doesn't matter [23:18] But if there are 3 others like it, that's more than 100ms. [23:19] and without the missing order_by, we are still loading people [23:19] so adding the order_by now is the right thing to do [23:19] to fix the core issue [23:19] OK, didn't notice that. [23:19] But still, 40ms is not negligible. [23:20] We should be getting the page's core query down that low if at all possible. [23:20] if a page takes say 1s round trip time to render it is [23:20] And no other query on the page has any business being more than 10ms. [23:20] wallyworld: In Australia things will be slow, sure. [23:20] But Australia is negligible. [23:21] Western US -> UK latency is ~130mx [23:21] ms [23:22] 40ms is a third of that. [23:22] i'm not arguing against optimisation. just the right time to do it [23:22] That's what LP said for the first 6 years. [23:22] Look where we are now :) [23:23] sigh. the page is already faster than most others. i'm more concerned with getting the right data displayed initislly [23:23] then we can optimise [23:23] i'd also rather initially follow established lp patterns [23:23] and see if they are good enough [23:23] before diverging [23:25] It depends on your definition of "good enough". [23:25] I argue that our traditional definition of good enough is not good enough. [23:25] if i can open my browser and load a page in < 1s that's good ebough for me [23:26] and worrying about 40ms at the expense of code divergence etc is not worth it [23:26] not saying we shouldn't fix that 40ms [23:26] but priorities... [23:28] We can very easily avoid that 40ms now with about two lines of code. [23:29] it's more than that since we would not be loading whole Persons anymore [23:29] so the model query needs to change as does the code to assemble the data for the view [23:31] Oh, I wasn't saying to avoid loading the persons at all. [23:31] That would indeed be a bit radical for such a small improvement right now. [23:31] But you're currently loading them twice. [23:31] getPillarShareeData takes a list of person, then queries them back out of the DB. [23:31] When it really just needs the IDs to match against the provided list. [23:33] that method can be called standalone [23:34] so it needs to be defined consistently with the getPillarSharees method [23:34] Ah, true, in this design. [23:35] I had envisaged that these would be a single method, using something like DecoratedResultSet to add the permissions into the sliced ResultSet. [23:35] Which would be roughly the same interface as the original, just more efficient and sliceable. [23:36] morning people [23:36] but we would lose the ability for users to have available that 2nd method if they wanted to use it [23:36] ohai bigjools [23:36] so we have done a lot of arguing over a drop in the ocean in terms of total time a user sees a page to render [23:37] wallyworld: But four drops in the ocean add more than 150ms to the page. [23:37] wallyworld: Which is more than the RTT for the vast majority of our users. [23:37] why 4? [23:38] It's just the number of 40ms-overhead operations that add up to >RTT. [23:38] but it's only one. and there'a lot more to rendering a page than RTT [23:38] Not this page. [23:39] mustache on the client takes a bit of time [23:39] That's a bug that can't be trivially fixed right now. [23:40] sure, so 40ms just doesn't matter right now [23:40] since other things swamp it [23:40] But then we replace mustache with handlebars globally. [23:40] And this page is still slow. [23:40] Because the server-side is crap. [23:41] not slow [23:41] And now we have to go and fix *every page in Launchpad* separately. [23:41] Because we knowingly wrote slow code. [23:41] Because it was dominated by slow client-side rendering. [23:41] we have different definitions of slow === matsubara is now known as matsubara-afk