/srv/irclogs.ubuntu.com/2012/03/21/#launchpad-dev.txt

wgrantStevenK: Hm, you didn't move the two constants?01:11
wgrantAFAICS you just renamed them.01:11
wgrantPRIVATE_BUG_TYPES etc.01:11
StevenKI was supposed to move them?01:11
wgrantI said they should be alongside the enum.01:12
wgrantPerhaps I was unclear.01:12
wgrantBut there's nothing bug-specific about them.01:12
StevenKwgrant: Sorry, I've moved them to registry.enums01:16
wgrantThanks.01:16
cody-somervillesinzui, That page seems to scroll really slowly in my Firefox.01:20
wgrantcody-somerville: For which project?01:21
cody-somervillebugsy01:21
wgrantApproximately how many rows does it have?01:22
cody-somervillea lot.01:23
wgrantUbuntu has nearly 20000, which is a lot.01:23
wgrantLaunchpad has a few hundred, which is a lot.01:23
wgrantWhat is a lot?01:23
cody-somervilleprobably less than 10001:24
wgrantAh01:25
wgrantNot a lot :)01:25
StevenKwgrant: I guess I should kill IBug.set{Private,SecurityRelated} and related gubbins?01:29
wgrantStevenK: Something like them is still necessary for the API.01:29
wgrant(and +secrecy)01:29
StevenKRargh, setPrivacyAndSecurityRelated is still exported.01:30
=== matsubara is now known as matsubara-afk
wallyworld_StevenK: you ocr?02:34
wgrantStevenK: Here, I made three code reviews, just for you :)02:49
wallyworld_and one from me02:51
wallyworld_https://code.launchpad.net/~wallyworld/launchpad/tri-state-sharee-picker-959784/+merge/9855602:51
wallyworld_it's mainly javascript too, and i know StevenK loves javascript :-)02:51
wallyworld_and if he does them soon, we can make the next buildbot run :-)02:52
StevenKI was reviewing my lunch.02:54
lifelessnom02:54
StevenKwgrant: Three? I can only see two.02:54
wgrantIt would help if I submitted the MP for the last one.02:55
StevenKHaha02:56
StevenKwgrant: All three done.03:02
StevenKwallyworld_: You're up now.03:02
wgrantStevenK: Thanks.03:02
StevenKwallyworld_: Three comments, the first two are not optional, the last one is.03:07
wallyworld_StevenK: thanks03:14
StevenKwallyworld: If you hold down the "Propose merge" button it brings up 3 or 4 spinners and then errors horribly03:51
wallyworldStevenK: so don't hold the button down :-P03:51
nigelblol03:52
huwshimiStevenK, 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:53
wallyworldhuwshimi: yes, or something like that. disable on click even03:54
StevenKNo, I think we need do it on mouseUp03:54
wallyworldyes, you are right03:54
* huwshimi thinks wallyworld missed the joke03:54
StevenKI didn't think you got mulitple mouseDown events if you held the button down03:54
wallyworldme wither03:55
wallyworldeither03:55
lifelessstub: wgrant: have the two of you spoken about not using FK's in the FACT tables ?03:55
wgrantNo, but indeed we should.03:56
wgrantstub: So I heard you like foreign keys.03:56
nigelb("Yo dawg")03:56
nigelbI just imagined wgrant say it with that added.03:56
stubwgrant: I shirley dooly03:57
wgrantstub: I'm not sure they're valuable on fact tables.03:58
wgrantAnd in some cases they're not possible.03:58
wgranteg. denormed arrays03:58
stubwgrant: Looks like they will be in 9.2 or 9.3 :)03:58
stubBut sure, if PG doesn't support it that is a good reason to not use that case.03:58
wgrantGiven that they're maintained by tested triggers from FKed columns, the only purpose they serve seems to be to cause slow writes :)03:58
wgrantOh really?03:58
wgrantHmm03:58
wgrantOh, I see a patch. Nice.03:59
stubI'm fine with not having them if the table is driven by triggers.03:59
stubIt is a harder decision to make if writes are distributed over more code, as we are more likely to screw up.04:00
wgrantRight. 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
wgrantIf app code is touching them, agreed.04:00
stubSo are the fact tables going to reference libraryfilealias (for, say, patches) or person?04:01
wgrantThey reference Person, but not LFA.04:02
wgrantWhich is slightly interesting.04:02
stubWill need some special well commented code in person merge then04:02
wgrantBut the triggers handle the person merge case fine.04:02
stubOk.04:02
stuboh, of course.04:02
stubis all fine04:03
wgrantThat array FKs thing is really nice, though.04:04
wgrantMight actually make them practical for normal use.04:05
=== wallyworld_ is now known as wallyworld
lifelesspoolie: hi05:11
lifelesswgrant: small arrays only probably, don't want them TOASTed (to be better than a separate table.. perhaps)05:12
wgrantlifeless: TOAST isn't too bad in some situations.05:15
lifelessits fine as long as you're not using an ORM that requests every field always05:15
wgrantAhem05:15
lifelessok05:15
lifelessso thats hyperbole05:15
lifelessbut not all that far off05:15
wgrant(fortunately the ORM isn't going near my array table :))05:15
wgrantAlso, isn't the TOAST threshold like 8KiB?05:16
wgrantThat's one large array.05:16
lifelessnah05:16
lifelessthat would be 2 pages05:16
poolielifeless, hi05:17
lifeless2K I think05:17
wgrantPages are 8KiB.05:17
pooliedoes lp actually dislike docstrings on tests, or does it just tend to not have them?05:17
lifelessbit of both05:18
lifelessthey aren't very useful, and the rules for docstrings are rather awkward for comments on many tests05:18
lifelessalso they tend to be redundant with well chosen test names05:18
StevenKI prefer a comment at the top of a test, but I may forget to add one.05:18
lifelesswgrant: I have no idea why I thought 4KiB05:19
wgrantlifeless: Because every other computing definition of "page" is 4KiB.05:19
lifelessyes05:19
lifelessThe 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 move05:20
lifelesshttp://www.postgresql.org/docs/8.4/static/storage-toast.html05:20
wgrantAh05:20
wgrantI concede.05:22
pooliei know unittest used to have stupid behaviour where it would not print the test name if there was a docstring05:24
pooliebut i'm pretty sure that's fixed in lp's test runner05:24
wgrantpoolie: I think our testrunner still does.05:24
wgrantHmm05:24
wgrantMaybe that was fixed.05:24
wgrantOr I'm thinking of another project.05:24
wgrantOh goody.05:24
wgrantcode.launchpad.net is magically faster now.05:24
wgrantAnd Bugs is slowly speeding up.05:24
wgrantwtf launchpad why are you doing this05:24
wgrantdon't get better without us doing anything :/05:24
* wgrant breaks the world with HTML5.05:30
lifelessstub: so wtf with bugtag.name05:36
lifelessstub: how was 8.4 working at all ?05:36
stublifeless: I have no idea05:36
lifelessstub: tests failed for you ?05:36
stublifeless: 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
stubYes, picked up with a failed test run. Testing my 9.1 ec2 image.05:37
wgrantThat code path should have been reached, though :/05:37
stubI'd seen it before, but thought I'd fixed it already.05:37
wgrantstub: That code is fairly new05:38
stubThere are still a few other tests failing I want to fix later today.05:38
wgrantAnd should really just be deleted.05:38
=== jtv is now known as jtv-afk
=== almaisan-away is now known as al-maisan
=== danilo_ is now known as danilos
stubsalgado's email re: conjoined bugs confuses me, and leads me to fail to discover any actual definition of a conjoined bug.07:42
stub    _NON_CONJOINED_STATUSES = (BugTaskStatus.WONTFIX, ) is probably what confuses me the most07:45
wgrantstub: That's the bit of the definition that nobody remembers.07:46
wgrantA bugtask conjoinment relationship has two halves: the master and the slave.07:47
wgrantThe master task is the development series task: a productseries, distroseries, or sourcepackage task.07:47
wgrantThe slave task is the non-series task: a product, distribution or distributionsourcepackage task.07:48
wgrantConjoinment implicitly occurs when there is a task for the development series.07:48
wgrantThe slave is no longer directly editable, instead being mirrored from the master.07:48
wgrant*Except* when the master is marked Won't Fix, in which case the conjoinment relationship is broken, and the slave is separate again.07:48
wgrantIt's like somebody tried to devise the most obscure set of rules they could think of.07:49
stubYer. Your definition makes sense. I suspect it would have benefited from rethinking the data model around those rules at the time.07:50
wgrantYou'd think.07:50
lifelesswell07:51
lifelessthe whole problem was one of model leaking07:51
lifelessnominations contribute to the pain too07:51
lifelessskaet would love only-series tasks to be implemented :)07:51
wgrantAnd most of the rest of the world would not :)07:52
lifelesswell07:52
lifelessI don't think thats true07:52
lifelessmost of the rest of the world wouldn't ever notice </pedantic>07:52
adeuringgood morning08:56
czajkowskialoha09:06
mabacgmb, hi there! are you up for reviewing a fix for me? :) https://code.launchpad.net/~linaro-infrastructure/launchpad/notify-workitems-changes/+merge/9823110:49
mabacI probably didn't mean gmb since it looks like he's not online. :)10:58
=== al-maisan is now known as almaisan-away
gmbmabac: I'm mobile ATM. Also, it's11:15
gmbNot11:15
gmbMy review day.11:15
mabacgmb, sorry, I just thought I'd bug you since the topic listed you as OCR. :)11:16
gmbmabac: Check http://dev.launchpad.net/ReviewerSchedule for details of who's on today.11:16
gmbIt's out of date. Apologies.11:16
mabacgmb, thanks! I didn't know about that page11:16
=== gmb changed the topic of #launchpad-dev to: https:/​/​dev.launchpad.net/​ | On call reviewer: - | Firefighting: - | Critical bugtasks: 4*10
gmbDoing that on my phone was hard :/11:17
czajkowski:/11:17
czajkowskigmb: ello ello11:17
mabacgmb, :) thanks11:17
gmbHi czajkowski.11:18
czajkowskigmb: fancy a UDS photo walk one of the evenings?11:18
gmbczajkowski: Definitely!11:18
czajkowskihop on the bart and walk around san fran take pics and grab foods?11:18
czajkowskiplan it in advance and get some numbers?11:18
czajkowskiand you cna plan a route :)11:18
czajkowskiknow there are a lotta photo folks in ubuntu could be fun11:18
czajkowskisomething different to do11:18
gmbHaha. 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
czajkowskiam gonna update the uds wiki page11:19
czajkowskiwill mail you in a bit11:19
gmbCool.11:19
czajkowskiand then get it posted to uds organisers :)11:19
* gmb afk to give someone a large amount of money11:20
czajkowskiif people know in advance then it wont clash with team dinners11:20
gmbczajkowski: Right; makes perfect sense.11:27
salgadostub, who's working on the private bugs overhaul you mentioned?12:31
stubCurtis' team12:32
stubsinzui: ^12:33
czajkowskipurple squad working their magic12:34
salgadocool, thanks stub12:37
wgrantsalgado: It's not so much as an overhaul as a complete redesign and rewrite from scratch.12:47
wgrantget_bug_privacy_filter is what you should be using, indeed.12:47
wgrantWe'll have to rewrite your query soon, though.12:47
salgadowgrant, ok, cool.  but why rewriting my query is going to be needed?12:48
=== matsubara-afk is now known as matsubara
wgrantsalgado: Because there's a new, faster bug search schema coming soon, and privacy is completely different.12:49
salgadooh, ok12:50
mabacI'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/9823113:07
rick_hmabac: he should be in soon.13:08
rick_hour stand up is in 3013:08
rick_hso will have to catch him after that probably13:08
mabacthanks rick_h13:08
=== 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
mabacabentley, good to see you. I hope you're happy to learn that I'm hoping to add to your work load this early ;)13:31
mabacabentley, https://code.launchpad.net/~linaro-infrastructure/launchpad/notify-workitems-changes/+merge/98231 is in need of a review13:32
abentleymabac: On The Phone13:32
czajkowskimabac: wow you really want this code reviewed!13:42
mabacczajkowski, 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:44
mabacabentley, sure no rush. really. I'm sorry for jumping on you like that13:45
sinzuijcsackett, I am indeed trying to get you branch merged.13:59
sinzuijcsackett, I decided to PQM landed it after getting another false error in ec2 test14:00
jcsackettsinzui: got it, so i should not attempt to reland on my end. :-P14:00
sinzuiThere is now a text conflict though14:00
jcsackettsinzui: if you have not pqm-landed it, i should be able to land on my end now.14:01
jcsackettor if you're already dealing with it, i'm happy to leave it to you, if you would prefer.14:01
sinzuijcsackett, Was fixing the conflict. let me try for 2 more minutes14:01
jcsackettsinzui: sure.14:01
sinzuijcsackett, I am going to run the pillar tests locally. You and Ian collided14:06
jcsackettsinzui: ok.14:06
abentleyadeuring: http://pastebin.ubuntu.com/892304/14:07
adeuringabentley: can't header you any more14:18
adeuringabentley: mumble instead?14:19
abentleyadeuring: Sure.14:19
sinzuijcsackett, your branch is merged.14:21
jcsackettsinzui: awesome, thanks. :-)14:21
jcsackettglad i sorted out what you were doing before lp-landing on my own. :p14:22
sinzuijcsackett, 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:24
jcsackettsinzui: that seems very reasonable, and having just updated from devel it all looks right to me.14:25
=== almaisan-away is now known as al-maisan
abentleymabac: Your functionality looks okay.14:32
abentleymabac: You are adding LOC.  Do you have permission to do that?14:32
mabacabentley, I hope this is considered offset by the tech debt payed off by salgado for the work items feature14:33
salgadoabentley, this is a bug fix, not a new feature, btw14:33
mabacabentley, so my answer is, yes I believe so14:33
abentleysalgado: Is there a blanket exemption for bug fixes?  I didn't think there was.14:34
mabacabentley, still this should have been part of the initial feature, just neglect on my part that it wasn't14:35
abentleymabac: Okay.14:35
abentleymabac: 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
czajkowskibac: if you get a chance would you mind looking over https://answers.launchpad.net/launchpad/+question/169663  please.14:36
salgadoabentley, 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 off14:37
abentleymabac: 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:38
mabacabentley, ok cool.14:39
abentleysalgado: No, there's no room for reviewers discretion.  waivers are granted by flacoste and lifeless.14:40
flacosteabentley: yes, salgado's credit is still good on that one14:42
salgadoflacoste, 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:47
abentleysalgado: No, there's no room for reviewers discretion.  waivers are granted by flacoste and lifeless.14:48
abentleymabac: Also 138-142, 148-149, 170-171, 185-190, 191-196, 205-206, 210-21314:48
abentleymabac: Is the lint clean?14:48
salgado<flacoste> abentley: yes, salgado's credit is still good on that one14:48
salgado<-- abentley has quit (Ping timeout: 264 seconds)14:48
mabacabentley, 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:54
abentleymabac: 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:55
mabacabentley, 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:57
abentleymabac: Please do.14:58
abentleyadeuring: The problem still exists, but storing job.fail as a local variable avoids it.15:06
adeuringabentley: ah, ok. So you were right that the problem is caused by late loading job.15:07
mabacabentley, I have pushed changes, I hope it looks better now15:10
abentleymabac: Thanks.  That's better.  At 57, 150, 172, 209, the line should be indented four spaces.15:13
bacczajkowski: 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:14
abentleymabac: 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:15
czajkowskibac: 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 ever15:16
bacczajkowski: yes, it is odd15:17
abentleymabac: 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
abentley?15:17
bacczajkowski: i do see his key is crazy old...199615:17
mabacabentley, yes. that's handled by change line 5615:18
abentleymabac: great.15:19
czajkowskibac: makes no sense we'd surely have others with the same issue15:19
mabacabentley, I've pushed another revision with the indentation fixes15:22
abentleymabac: I think your comment in tests, "In production this notification is fired by lazr.restful..." is now stale.15:24
abentleymabac: Also, you should add a description of each test-- either a docstring or a comment.15:25
abentleymabac: Do the tests still need transaction.commit?15:25
mabacabentley, 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 it15:25
mabacabentley, they do. and I've followed an existing example where it's done exactly like that15:26
mabacabentley, ack on the docstrings15:26
abentleymabac: But "this notification is fired by lazr.restful" is true, but not complete, now that it's also fired in another location.15:26
mabacabentley, true. I'll fix it15:27
abentleymabac: r=me with those changes.15:28
mabacabentley, thank you very much!15:29
abentleymabac: np.  Sorry for the delay.15:29
mabacabentley, that was very quick so I'm very happy :)15:29
abentleymabac: line 57 still looks wrong.15:31
mabacabentley, sorry about that, I had forgotten to save that file15:32
mabacabentley, fixed and docstrings added15:34
abentleymabac: great.  Do you need me to land this for you?15:35
mabacabentley, if you would. thanks!15:35
mabacabentley, that is, yes please :)15:36
abentleymabac: okay, will do.15:37
mabacabentley, thank you15:37
sinzuijcsackett, I sent you an image to add to your mockup directoy15:39
sinzuijcsackett, 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 permissions15:42
jcsackettsinzui: rereading your email now, sorry. :-P15:42
salgadodanilos, mabac, the call is in 20 minutes.  are you guys joining?15:43
jcsackettsinzui: pushing it up to the directory now.15:43
sinzuithanks.15:44
=== al-maisan is now known as almaisan-away
danilossalgado, I am, mabac won't be able to15:46
danilosmrevell, salgado: hi, I suppose mumble or hangout will do?15:54
mrevellsalgado, flacoste, danilos: Mumble is fine.15:54
=== abentley_ is now known as abentley
=== almaisan-away is now known as al-maisan
mrevellsalgado, flacoste: We're in the Product channel on Mumble16:00
danilosflacoste, joining us?16:07
mrevelldanilos, salgado: https://dev.launchpad.net/Projects/WorkItems/16:09
=== 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
flacostemrevell: sorry, i had a dr appointment, i forgot to tell you17:54
mrevellflacoste, 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.17:55
flacosteok18:05
czajkowskigmb: https://wiki.ubuntu.com/UDS-Q/OtherEvents18:30
gmbczajkowski: Ah, thankee. I've already turned up one or two potential routes. Really depends what we're after (landmarks vs interestingness).18:45
czajkowskigmb: factor in a food stop18:46
gmbczajkowski: Right. And also travel time from t'hotel, which, this being the BART, could be substantial.18:46
gmbAnyroadup, I'm sure we'll come up with something.18:47
czajkowskigmb: 30 mins I think18:47
gmbczajkowski: Yeah, it's not so much the travel time as the waiting-for-a-train time.18:48
czajkowskinods18:48
lifelessflacoste: hi20:37
flacostehi lifeless20:37
lifelessflacoste: did you see skaets in-line request for escalation on the audit trail bug ?20:38
flacosteyes20:38
lifelesssalgado: hi, when is your EOD ?20:38
lifelessflacoste: cool20:38
flacostelifeless: i replied to her via irc, but i think i'll need to follow-up more20:38
flacostelifeless: basically, i don't think we have time to do it before the release20:38
flacosteso not sure it's worth escalating at this point20:38
lifelessyeah, I had a private chat too20:39
lifelessthe bug wasn't on our management radar20:39
salgadolifeless, in about 20 minutes, why?20:39
lifelesssalgado: I thought I might try and help you with this bug search thing20:39
lifelesssalgado: if you would like to talk about it, irc or voice or whatever20:39
salgadolifeless, sure, mumble, hangout or something else?20:40
lifelessskype or hangout work best for me20:41
lifelessmumbles lack of echo cancellation -> pain20:41
lifelessflacoste: if you need anything from me (that I'm not already doing :P) on that bug, let me know.20:41
flacostei will20:42
salgadolifeless, agreed. starting a hangout now.20:42
lifelesswow, unity's alt-tab is reallly hard to use20:43
lifelessjust breaking me all the time atm20:43
salgadolifeless, http://paste.ubuntu.com/894246/20:44
salgadolifeless, https://dev.launchpad.net/Projects/WorkItems21:00
pooliehas anyone else seen an error about22:15
poolie2012-03-21 06:14:39 UTC FATAL:  could not access private key file "server.key": Permission denied22:15
poolierunning current lp?22:15
wgrantpoolie: No. Possibly an SSH server key?22:26
pooliegoogle suggests something about an ssl key22:27
pooliewhich would make more sense for postgres22:27
poolie(i should have mentioned that's where it is)22:27
poolieprobably something local22:28
wgrantOh, if that's postgres, then yes, it's probably postgres :)22:30
sinzuiwallyworld, Surely there is something in this list that irks you: https://bugs.launchpad.net/launchpad/+bugs?field.tag=trivial22:53
nigelbMorning.22:58
poolieif i grant a team access to a pppa, will they all get spam^Wmailed?23:01
poolieo/ sinzui23:01
poolieo/ nigelb23:01
lifelessyes23:01
nigelbHey poolie! Got a new motorbike yet? :)23:01
poolienot yet, but i did get the payout for the old one23:02
pooliei'm not going to get the chance to ride much for the next while so i might wait23:02
sinzuihi poolie23:02
nigelbOh, nice.23:02
* huwshimi goes digging to find out who broke the homepage23:02
poolielifeless, that 'yes' was to me?23:03
poolie:/23:03
sinzuipoolie, do you think the emails should go to the team admins? or none at all?23:03
poolienot sure23:04
pooliemaybe it would be nice to have a tick box for "mail people to tell them they have access", like google docs does for instance23:04
poolieor, just silently give them access23:04
pooliepresuming they'll find out some other way23:04
wgranthuwshimi: You mean the header being wider than the rest of the page?23:04
poolieit doesn't seem like a thing where there's a strong reason they _must_ be told23:04
huwshimiwgrant: Yeah...23:04
wgranthuwshimi: sinzui removed locationless a while ago.23:05
wgrantThat's what changed it.23:05
wgrantrevno: 14867 [merge]23:05
wgranttimestamp: Sat 2012-02-25 07:26:36 +000023:05
huwshimiwgrant: Oh, so this has been live for a while!?23:05
wgrantmessage:23:05
wgrant  [r=bac][bug=432889,435832,580240,938889] Update error pages to look23:05
wgrant        and read like other Lp pages; locationless is gone.23:05
sinzuipoolie, 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 arrive23:06
poolieindeed, i guess that connects to a more generally nuanced view of notification23:06
poolieso "tell them now" would perhaps23:06
pooliemean23:06
poolie"it's important they know now", which would push it up to being sent by mail23:06
poolieor, maybe there should just be a per-user "tell me when i get a new ppa" flag23:07
poolies/tell/mail23:07
poolieanyhow, generic notifications ++23:07
sinzuiI pioneered a mechanism to show notifications from non-browser code last week. It is possible to store them then unqueue them when the user arrives23:07
wgrantStevenK: Should 933759 be in QA-Landing?23:07
lifelesspoolie: yes23:08
StevenKwgrant: So I'd like to add information_type to branch's model code and then use that branch to close the bug.23:09
wgrantStevenK: Mmm.23:10
wgrantStevenK: It's not useful yet, and won't be for a while.23:10
StevenKwgrant: Or we can just close it saying the column exists and other bugs deal with switching to it.23:11
wgrantYes.23:11
wgrantThat was my intent.23:11
StevenKDoit23:11
wallyworldwgrant: 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:11
wallyworldyou mentioned not to do that but i think it's the way to go23:12
wallyworldthat's effectively how it was before i split the query23:12
wgrantwallyworld: You shouldn't be loading people in that query.23:13
wgrantGiven how slow loading people is.23:13
wgrantI guess you could do it, given that it's batched now.23:13
wallyworldyes i think so23:13
wallyworldit's only a few records eg 7523:13
wallyworldwe can optimise later if really needed23:13
wgrantThat's still around 40ms :/23:14
wallyworldthat's negligable23:14
lifelessmm23:15
lifelessI think we need to recalibrate our speed expectations :)23:15
lifeless40ms is actually quite slow23:16
wgrantYes.23:16
wgrantThat's nearly 10% of the average render time of Product:+index23:16
wgrantWhich is still slow.23:16
wallyworldin the context of a round trip it's ok surely23:16
wallyworldand the rest of lp loads people in queries as needed23:16
wgrantSure, but the rest of LP is fucking terrible.23:17
wgrantThis stuff doesn't have to be :)23:17
wallyworldand we're talking max batch_size records23:17
wallyworldlet's just get sorting fixed first and then optimise if needed23:17
wallyworldi sure wouldn't notice if a page rendered 40ms slower23:17
wgrantIt's a 40ms delay for a non-significant query.23:18
wgrantOn its own it's not terrible.23:18
wallyworldso? in the content of the overall page render time, it doesn't matter23:18
wgrantBut if there are 3 others like it, that's more than 100ms.23:18
wallyworldand without the missing order_by, we are still loading people23:19
wallyworldso adding the order_by now is the right thing to do23:19
wallyworldto fix the core issue23:19
wgrantOK, didn't notice that.23:19
wgrantBut still, 40ms is not negligible.23:19
wgrantWe should be getting the page's core query down that low if at all possible.23:20
wallyworldif a page takes say 1s round trip time to render it is23:20
wgrantAnd no other query on the page has any business being more than 10ms.23:20
wgrantwallyworld: In Australia things will be slow, sure.23:20
wgrantBut Australia is negligible.23:20
wgrantWestern US -> UK latency is ~130mx23:21
wgrantms23:21
wgrant40ms is a third of that.23:22
wallyworldi'm not arguing against optimisation. just the right time to do it23:22
wgrantThat's what LP said for the first 6 years.23:22
wgrantLook where we are now :)23:22
wallyworldsigh. the page is already faster than most others. i'm more concerned with getting the right data displayed initislly23:23
wallyworldthen we can optimise23:23
wallyworldi'd also rather initially follow established lp patterns23:23
wallyworldand see if they are good enough23:23
wallyworldbefore diverging23:23
wgrantIt depends on your definition of "good enough".23:25
wgrantI argue that our traditional definition of good enough is not good enough.23:25
wallyworldif i can open my browser and load a page in < 1s that's good ebough for me23:25
wallyworldand worrying about 40ms at the expense of code divergence etc is not worth it23:26
wallyworldnot saying we shouldn't fix that 40ms23:26
wallyworldbut priorities...23:26
wgrantWe can very easily avoid that 40ms now with about two lines of code.23:28
wallyworldit's more than that since we would not be loading whole Persons anymore23:29
wallyworldso the model query needs to change as does the code to assemble the data for the view23:29
wgrantOh, I wasn't saying to avoid loading the persons at all.23:31
wgrantThat would indeed be a bit radical for such a small improvement right now.23:31
wgrantBut you're currently loading them twice.23:31
wgrantgetPillarShareeData takes a list of person, then queries them back out of the DB.23:31
wgrantWhen it really just needs the IDs to match against the provided list.23:31
wallyworldthat method can be called standalone23:33
wallyworldso it needs to be defined consistently with the getPillarSharees method23:34
wgrantAh, true, in this design.23:34
wgrantI had envisaged that these would be a single method, using something like DecoratedResultSet to add the permissions into the sliced ResultSet.23:35
wgrantWhich would be roughly the same interface as the original, just more efficient and sliceable.23:35
bigjoolsmorning people23:36
wallyworldbut we would lose the ability for users to have available that 2nd method if they wanted to use it23:36
nigelbohai bigjools23:36
wallyworldso we have done a lot of arguing over a drop in the ocean in terms of total time a user sees a page to render23:36
wgrantwallyworld: But four drops in the ocean add more than 150ms to the page.23:37
wgrantwallyworld: Which is more than the RTT for the vast majority of our users.23:37
wallyworldwhy 4?23:37
wgrantIt's just the number of 40ms-overhead operations that add up to >RTT.23:38
wallyworldbut it's only one. and there'a lot more to rendering a page than RTT23:38
wgrantNot this page.23:38
wallyworldmustache on the client takes a bit of time23:39
wgrantThat's a bug that can't be trivially fixed right now.23:39
wallyworldsure, so 40ms just doesn't matter right now23:40
wallyworldsince other things swamp it23:40
wgrantBut then we replace mustache with handlebars globally.23:40
wgrantAnd this page is still slow.23:40
wgrantBecause the server-side is crap.23:40
wallyworldnot slow23:41
wgrantAnd now we have to go and fix *every page in Launchpad* separately.23:41
wgrantBecause we knowingly wrote slow code.23:41
wgrantBecause it was dominated by slow client-side rendering.23:41
wallyworldwe have different definitions of slow23:41
=== matsubara is now known as matsubara-afk

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!