| lifeless | flacoste: http://blog.mountaingoatsoftware.com/agile-succeeds-three-times-more-often-than-waterfall | 00:00 |
|---|---|---|
| StevenK | wgrant: I should fix security.cfg in the code branch, or the db branch? | 00:03 |
| wgrant | StevenK: You can do it in the DB branch, so might as well do it there. | 00:06 |
| wgrant | In fact, you have to do it there. | 00:06 |
| wgrant | Because of person foreign keys. | 00:06 |
| StevenK | Hmm, so I have to drop the column from person too | 00:07 |
| StevenK | That is going to be fun. | 00:07 |
| wgrant | Hm? | 00:07 |
| StevenK | Oh, it's a join, not a column | 00:07 |
| wgrant | Yes. | 00:08 |
| rick_h | StevenK: ping | 00:08 |
| StevenK | rick_h: O hai | 00:08 |
| rick_h | StevenK: hey, I started landing the test fixes | 00:08 |
| StevenK | I saw. | 00:08 |
| StevenK | I think. | 00:08 |
| rick_h | StevenK: in my dev work I noticed that make jsbuild doesn't update the build dir files | 00:08 |
| rick_h | So I saw combobuild | 00:09 |
| rick_h | but that didn't update an existing file either | 00:09 |
| rick_h | I kept having to make clean && make to get the build dir updated to work on a js file as I worked | 00:09 |
| rick_h | which was a tad slow as you can imagine | 00:09 |
| rick_h | I tried to see if the combobuild didn't work right because it wasn't listed in PHONY, but that wasn't it. | 00:09 |
| rick_h | I didn't get to look at all of it before EOD | 00:09 |
| StevenK | bin/combo-rootdir will exit with a message if build/js/lp exists | 00:10 |
| rick_h | StevenK: right, it basically ran without cp'ing the files | 00:10 |
| rick_h | StevenK: we need some way to work on a .js file and getting it rebuilt like make jsbuild | 00:10 |
| StevenK | rick_h: Right, so now that icing depends on the YUI directory inside build/js, I can't just blow away the entire directory with impunity like I used to. | 00:11 |
| rick_h | StevenK: right, I have on my todo to work on a script you can run using pyinotify to auto rebuild changed files | 00:12 |
| rick_h | but have things keeping me from getting to it yet | 00:12 |
| rick_h | plus, that's going to be a pain since we've got build exceptions :/ | 00:12 |
| StevenK | rick_h: Right, so I've not come up with a good plan yet. | 00:13 |
| rick_h | StevenK: ok, well that's part of it. Wanted to make sure I wasn't expecting something to work that wasn't | 00:13 |
| rick_h | sounds like it's on the todo list then | 00:13 |
| rick_h | which is ok, now that I know | 00:13 |
| StevenK | I think I can delete everything but yui and then cope if yui is a symlink | 00:14 |
| rick_h | yea, I was thinking of just making a temp make command that blew away /build/lp and rebuilt it | 00:14 |
| StevenK | Meh, don't do that | 00:15 |
| StevenK | Fix combo-rootdir to not be so naive :-) | 00:15 |
| rick_h | it was a sledgehammer solution :) | 00:15 |
| StevenK | rick_h: I'll look at fixing combo-rootdir today | 00:15 |
| rick_h | yea, I'll look at it then. Thanks for filling me in | 00:15 |
| rick_h | ok, well no big deal. No one's really using it yet | 00:16 |
| * StevenK is destroying code and tables | 00:16 | |
| rick_h | StevenK: so qas ok? Is it setup I can run combo loader on qas if I set the FF? | 00:16 |
| StevenK | rick_h: You can't set the flag, and no, it's still waiting for that RT. | 00:16 |
| rick_h | ok, cool | 00:17 |
| lifeless | wgrant: I have some bad news w/respect heat | 00:23 |
| lifeless | wgrant: we're missing some update cases. | 00:23 |
| lifeless | wgrant: I suspect dupes, subscribers and affects-me-too are not updating the bug record heat | 00:24 |
| wgrant | lifeless: Are too. | 00:28 |
| wgrant | at least affects-me-too is. | 00:28 |
| wgrant | Subscribers work too | 00:28 |
| wgrant | And I can't test duping non-destructively. | 00:28 |
| StevenK | wgrant: Easy review https://code.launchpad.net/~stevenk/launchpad/kill-entitlements/+register-merge | 00:29 |
| StevenK | BAH | 00:29 |
| StevenK | When did that stop working? | 00:29 |
| wgrant | Oh, you're afflicted too? | 00:29 |
| wgrant | wallyworld_: ^^ | 00:29 |
| wgrant | We have a sane reproducer. | 00:29 |
| StevenK | wgrant: https://code.launchpad.net/~stevenk/launchpad/kill-entitlements/+merge/92888 | 00:29 |
| lifeless | wgrant: really? oh good. I was watching heat fail to change on prod when doin stuff the other day | 00:30 |
| wallyworld_ | wgrant: what's up? | 00:30 |
| wgrant | wallyworld_: The MP bug | 00:30 |
| wgrant | We have a reproducer who is not stupid :) | 00:30 |
| wallyworld_ | bug nr? | 00:30 |
| wgrant | Bug #929422 | 00:31 |
| _mup_ | Bug #929422: Fails to refresh the URL when making a merge request <Launchpad itself:Triaged> < https://launchpad.net/bugs/929422 > | 00:31 |
| StevenK | That's it | 00:31 |
| wgrant | Apparently it replaces the page content inline? | 00:31 |
| wgrant | That sounds extremely evil. | 00:31 |
| StevenK | It should stop doing so. | 00:32 |
| wallyworld_ | if you can come up with a solution that can avoid replacing the page content, i'm all ears | 00:33 |
| wgrant | wallyworld_: Redirect! | 00:33 |
| wgrant | window.location = whatever | 00:34 |
| wallyworld_ | isn't that another server request though | 00:34 |
| StevenK | It used to redirect and it was fine! | 00:34 |
| wgrant | wallyworld_: Don't you have to make the request to get the content anyway? | 00:34 |
| wgrant | It will just be a normal navigation request rather than an AJAX one. | 00:34 |
| wallyworld_ | hard to explain - an xhr request is made and it may error so we need to stay on the current page | 00:35 |
| wgrant | Sure | 00:35 |
| wallyworld_ | so that if there is an error | 00:35 |
| wallyworld_ | if can be displayed | 00:35 |
| wgrant | And when it succeeds it will give a 303, right? | 00:35 |
| wallyworld_ | atm it gives 200 | 00:35 |
| wgrant | Or 201 or something like that. | 00:35 |
| StevenK | wallyworld_: The URL can updated via JS | 00:35 |
| * wgrant reads the code. | 00:36 | |
| wallyworld_ | right, so that's all i need to do | 00:36 |
| wgrant | No | 00:36 |
| wgrant | That's evil. | 00:36 |
| wallyworld_ | i just forgot to update the url via js | 00:36 |
| wallyworld_ | why? | 00:36 |
| wgrant | We should avoid it if we can; it's not widely supported. | 00:36 |
| StevenK | wgrant: Thank you for the +1 | 00:36 |
| wallyworld_ | is it part of the standards? | 00:36 |
| wgrant | It's part of HTML5 | 00:36 |
| wallyworld_ | so only ie < 8 or something will have an issue? | 00:37 |
| wgrant | wallyworld_: No, not even IE9 supports it. | 00:41 |
| wallyworld_ | sigh. i hate ie | 00:41 |
| wgrant | It's a new API and what you're doing is reasonably evil. | 00:41 |
| wallyworld_ | fsvo | 00:41 |
| wgrant | Replacing the page content entirely is unprecedentedly evil. | 00:42 |
| wgrant | That is not arguable :) | 00:42 |
| wallyworld_ | i'll take your word for it i suppose | 00:42 |
| wallyworld_ | i mean, it's merely doing what the browser does when it gets stuff to render | 00:43 |
| wgrant | document.write is evil :) | 00:43 |
| wallyworld_ | why? it's a public api, or? | 00:43 |
| wgrant | There are lots of evil public APIs. | 00:44 |
| wallyworld_ | so the issue is that the lp form infrastructure returns the html resulting from doing a form submit, and the xhr call needs to do something with this data or else just throw it away and waste the rendering effort | 00:44 |
| wgrant | I had assumed that you used an API call. | 00:44 |
| wgrant | I didn't realise until a few minutes ago that you actually POSTed to +register-merge. | 00:45 |
| lifeless | so, FWIW | 00:45 |
| lifeless | I think avoiding roundtrips is good; but is this actually doing that ? | 00:45 |
| wallyworld_ | yes | 00:45 |
| lifeless | in which case, the UI impact is a regression, but it is otherwise tolerable, at least in the absence of e.g. doing the form via *stache | 00:46 |
| wallyworld_ | i posted to +register-merge to reuse all the existing code and avoid writing a bunch of new stuff and keep easy backward compat with non js browsers | 00:47 |
| lifeless | I heartily approve | 00:48 |
| wgrant | I approve of eliminating roundtrips, but I disapprove of introducing yet another non-standard way of doing forms. | 00:48 |
| wgrant | Particularly one that is fragile and uses widely discouraged APIs like document.write. | 00:48 |
| wallyworld_ | except i need to figure out a non evil way to handle the form submission via the xhr call without adding a rt and wasting rendering effort | 00:49 |
| wallyworld_ | maybe a server side redirect? | 00:49 |
| lifeless | thats what LP normally does | 00:49 |
| lifeless | Are you sure the xhr call isn't following it itself? | 00:49 |
| wgrant | There's still likely to be a round-trip, but it's not too terrible for this sort of page. | 00:49 |
| wallyworld_ | it would cause the server side rendering to be duplicated | 00:49 |
| wallyworld_ | perhaps | 00:49 |
| wgrant | The way we normally avoid duplication is by not POSTing to the view. | 00:49 |
| wgrant | But using an API instead. | 00:50 |
| lifeless | did you set out to eliminate roundtrips, or do something else? | 00:50 |
| wgrant | That's how all the other AJAX forms work, TTBOMK. | 00:50 |
| wallyworld_ | lifeless: i set out to convert what was a bog standard html form into a page that allowed the server to check for issues and send back an error message for display without leaving the page | 00:51 |
| wallyworld_ | and still act like a form submission on success | 00:51 |
| wallyworld_ | and live within the lp form infrastructure | 00:51 |
| lifeless | so, neither fish nor fowl :) | 00:52 |
| wallyworld_ | yeah, guess so | 00:52 |
| wallyworld_ | wanted to avoid too much new code / change etc | 00:52 |
| lifeless | so, deconstructing this situation | 00:52 |
| wallyworld_ | perhaps should look at using an api | 00:52 |
| StevenK | There is one? | 00:53 |
| lifeless | the new thing here was doin it in the picker | 00:53 |
| wallyworld_ | the work was already 5 or 6 branches long | 00:53 |
| StevenK | IBranch.createMergeProposal() | 00:53 |
| wgrant | eg. AJAX milestone creation is the normal form mapped to the API | 00:53 |
| lifeless | you could have, for instance, let the picker choose anyone, and show an error on submission | 00:53 |
| lifeless | would not have been as nice | 00:53 |
| wallyworld_ | lifeless: the picker does a check, but the submission also needs to do a check | 00:53 |
| lifeless | wallyworld_: I know | 00:53 |
| lifeless | wallyworld_: the picker check is redundant | 00:54 |
| wallyworld_ | so i did both | 00:54 |
| wallyworld_ | lifeless: yes, but it gives a nicer user experience | 00:54 |
| lifeless | indeed | 00:54 |
| lifeless | which I applaud | 00:54 |
| wallyworld_ | catch the error early | 00:54 |
| wallyworld_ | in the workflow | 00:54 |
| wallyworld_ | so, i needed a way to hit submit button but not leave the page and refresh if there is an issue, hence i made the xhr call, but the lp form stuff gives back a 200 with the new html for the resulting page on success | 00:56 |
| wallyworld_ | hence my use of document.write to render - it's the same data over the wire after all | 00:56 |
| lifeless | wallyworld_: I don't quite follow that last bit; why couldn't it just do what our other forms do - render with an error? | 00:57 |
| lifeless | (I'm not sayin that is good or optimal; just trying to understand) | 00:57 |
| wallyworld_ | lifeless: sure. there is no single field in error - it's a confirmation popup | 00:58 |
| wallyworld_ | not an error | 00:58 |
| wallyworld_ | and the form error stuff is a page refresh | 00:58 |
| wallyworld_ | which i didn't want | 00:58 |
| wallyworld_ | the user sees the confirmation popup and hits yes and a normal submission then occures | 00:58 |
| wallyworld_ | or if no, the popup goes away and they keep editing the mp | 00:59 |
| lifeless | I understand, but how is that different to a fresh page with a 'please click here to ok adding the user as a subscriber to ...' ? | 00:59 |
| wallyworld_ | yuk. | 00:59 |
| wallyworld_ | better , more web 2.0 experience | 00:59 |
| wallyworld_ | i didn't think doing a separate confirmation html page was a good thing to implement | 01:00 |
| wallyworld_ | so if no objections, let me see if a server side redirect works. might double render, not sure, but better than the current bug | 01:02 |
| lifeless | StevenK: can you check with webdev toolkit and see if a 30x is returned from the appserver? | 01:05 |
| StevenK | ... better? | 01:05 |
| StevenK | I'll be putting up an MP shortly | 01:05 |
| lifeless | wallyworld_: so, as a datapoint, *all* our forms today double the same form as the confirmation page for all data entry issues | 01:05 |
| lifeless | wallyworld_: e.g. with red boxes around bad data etc | 01:05 |
| lifeless | wallyworld_: fitting in with that may well have been much less effort, and certainly would have been more consistent | 01:06 |
| wallyworld_ | lifeless: but here, there is no bad data | 01:06 |
| wallyworld_ | there's no error | 01:06 |
| lifeless | wallyworld_: model it as a default-hidden field and there is | 01:06 |
| lifeless | that said, what we do today isn't all that nice | 01:07 |
| lifeless | it is however js-free | 01:07 |
| wallyworld_ | i was trying to avoid propagating the non-nice stuff | 01:07 |
| wallyworld_ | and we are allowed to do js only stuff now, right? | 01:08 |
| lifeless | that is still vague, sadly. | 01:08 |
| lifeless | we need to pin down the exact non-js use cases. | 01:08 |
| lifeless | there are, AFAIK, three: | 01:08 |
| wallyworld_ | well, too late, horse has well and truly bolted | 01:08 |
| lifeless | - login | 01:08 |
| lifeless | - apport bug filing | 01:08 |
| lifeless | - google indexing (all stuff we want indexed must have a server side render) | 01:09 |
| lifeless | but, TTBOMK we haven't got a solid ratification around that bein the complete set, yet. | 01:09 |
| wallyworld_ | lifeless: yes, it was extra work, but you need to do that to establish a new way forward to get away from the old, "bad", stuff, even if we decide we want to do it another way, we need to blaze a trail | 01:09 |
| wallyworld_ | see what works, what doesn't | 01:10 |
| lifeless | sure | 01:10 |
| lifeless | I haven't, I hope, been critical so far | 01:10 |
| wallyworld_ | oh, no not at all | 01:10 |
| lifeless | just trying to ensure I have a clear understanding | 01:10 |
| wallyworld_ | i appreciate a robust discussion, especially since i broke stuff :-( | 01:10 |
| wallyworld_ | i have a thick skin :-) | 01:11 |
| wallyworld_ | i do like the new behaviour, so would like to make that work rather than reverting to a confirmation page | 01:11 |
| StevenK | You have to, to be a Queenslander | 01:11 |
| wallyworld_ | :-P | 01:11 |
| wgrant | Damn, beat me to it. | 01:11 |
| lifeless | anyhow | 01:14 |
| lifeless | so, here is my take on it | 01:14 |
| lifeless | we probably need two forms of forms | 01:14 |
| lifeless | the bug filing / login case | 01:14 |
| lifeless | everything else | 01:14 |
| lifeless | we -don't- need server side rendering of forms, google doesn't need to index forms | 01:15 |
| lifeless | the current approach for populating and validating forms / form widgets / etc is API calls, not view calls. | 01:15 |
| lifeless | I'd rather we push on that unless we think its flawed, than overload the behaviour of server side forms | 01:16 |
| lifeless | which this seems to do to me | 01:16 |
| wallyworld_ | lifeless: validation is done via form submission | 01:16 |
| wallyworld_ | the lp form view converts the post params into objects etc and then calls validate | 01:17 |
| lifeless | wallyworld_: why? I mean, I know that is what you did, but API calls that mutate/create do their own validation | 01:17 |
| wallyworld_ | i'm relying on the standard lp form submission code to get the form data and convert to model objects | 01:18 |
| wallyworld_ | using the schema | 01:18 |
| wgrant | The API does that too. | 01:18 |
| wallyworld_ | yes. but i also then resuse the exisiting form validation callbacks | 01:19 |
| wallyworld_ | but using the standard submit | 01:19 |
| wgrant | Aren't they on the API too? | 01:19 |
| wallyworld_ | s/but/by | 01:19 |
| === nhandler_ is now known as nhandler | ||
| wallyworld_ | they are? how? one defines validate() on LaunchpadFormView | 01:20 |
| lifeless | the parameters are typed | 01:20 |
| lifeless | with schema validators attached to that (e.g. Choice()) | 01:20 |
| wallyworld_ | sure, i'm not talking about param conversion, but validation | 01:20 |
| wgrant | That should all be in the model. | 01:20 |
| wgrant | The view is just meant to present it as HTML. | 01:21 |
| wallyworld_ | the LaunchpadFormView subclass offers a validate() method | 01:21 |
| wallyworld_ | which people use | 01:21 |
| wallyworld_ | and then call field.setError | 01:21 |
| wgrant | Sure, but that should only be for formatting model errors as HTML. | 01:21 |
| wgrant | Since the model should be doing the validation, not the form. | 01:21 |
| wgrant | s/form/view/ | 01:21 |
| wallyworld_ | should be but doesn't always | 01:22 |
| wallyworld_ | i'll have to recheck this particular case | 01:22 |
| wallyworld_ | to see what it does | 01:22 |
| lifeless | wallyworld_: if the model does not do the validation, that is a bug | 01:22 |
| wgrant | Exactly. | 01:22 |
| wallyworld_ | then we have lots of bugs i think | 01:22 |
| wgrant | Because there is an API for this. | 01:22 |
| lifeless | wallyworld_: We have had many OOPSes - criticals - due to exactly that. | 01:22 |
| lifeless | this discussion has been good, I think I can suggest a path now | 01:23 |
| lifeless | (the one above), will send to list | 01:23 |
| wallyworld_ | lifeless: wgrant: the RegisterMergeProposalView does do it's validation in the view - so bug :-( | 01:23 |
| wallyworld_ | there is no well defined pattern that is in common use across lp for this | 01:25 |
| lifeless | well, I beg to differ | 01:25 |
| lifeless | we have two patterns and a bunch of sometimes poorly migrated forms | 01:25 |
| lifeless | the migration *should* have been to move all validation to API's | 01:26 |
| wallyworld_ | the code i look at often suggests otherwise :-) | 01:26 |
| lifeless | so there is an expected layout | 01:26 |
| lifeless | If there is an API, it *must* do its own validation. | 01:26 |
| wallyworld_ | agreed | 01:26 |
| lifeless | If there is a view and an API, the view should catch appropriate errors from the model, rather than duplicating validation code | 01:27 |
| wallyworld_ | yep, but mostly don't | 01:27 |
| lifeless | If there is a view and no API, its old unmigrated code. | 01:27 |
| lifeless | wallyworld_: -> lots of bugs | 01:27 |
| wallyworld_ | yep | 01:27 |
| wallyworld_ | lifeless: would you consider it a bug if a factory method makeFooBar() caused a side effect of leaving behind an admin login such that a check_permission in a test erroneously passes? | 01:35 |
| StevenK | I would. | 01:35 |
| wgrant | I ran into a similar thing last week. | 01:35 |
| wgrant | Except it was logging out. | 01:35 |
| * wallyworld_ stabs factory | 01:35 | |
| wgrant | I was tempted to wrap the factory in an interaction preserver. | 01:36 |
| StevenK | Fix the factory to do with celebrity_logged_in(): | 01:36 |
| wallyworld_ | in this case, makeRegistryExpert() is evil | 01:36 |
| wallyworld_ | yes, will fix, just wanted to check | 01:36 |
| * wallyworld_ wonders how many tests will fail now | 01:36 | |
| StevenK | wallyworld_: You will probably get fallout | 01:36 |
| StevenK | Ah, you guessed :-) | 01:36 |
| * wallyworld_ sighs | 01:36 | |
| StevenK | Welcome to Launchpad. | 01:36 |
| wallyworld_ | is it too late to run away? | 01:37 |
| wallyworld_ | StevenK: what's bad is that the method just above uses the correct pattern, so somehow that was not noticed | 01:38 |
| lifeless | wallyworld_: yes; an undocumented sideeffect is almost certainly a bug | 01:39 |
| StevenK | lifeless: Yes, it returns a 303. | 01:39 |
| lifeless | StevenK: thanks | 01:40 |
| lifeless | wallyworld_: ^ :) | 01:40 |
| lifeless | wallyworld_: server side is doing the right thing | 01:40 |
| wallyworld_ | for the register mp form? | 01:40 |
| lifeless | yes | 01:41 |
| StevenK | POST => 303 ; GET <MP ID> => 200 | 01:41 |
| StevenK | And then Firebug refreshes the Net panel, because it's evil. | 01:41 |
| wallyworld_ | ok, so if the xhr call gets back stuff with code=303 and content type = text/html and responseText = the page html, what do i do? | 01:42 |
| wallyworld_ | i could have sworn i got code = 200 | 01:42 |
| lifeless | you do | 01:43 |
| lifeless | the browser follows the 303 | 01:43 |
| wallyworld_ | ah right, i remember now | 01:43 |
| lifeless | question your assumptions, always :) | 01:43 |
| wallyworld_ | yes, i did figure that out | 01:43 |
| StevenK | wgrant: https://code.launchpad.net/~stevenk/launchpad/smarter-combo-rootdir/+merge/92897 | 01:43 |
| wallyworld_ | i had ust forgotten | 01:43 |
| wallyworld_ | but i now recall my dilema | 01:43 |
| wallyworld_ | the server side 303 is mapped to a 200 by the browser | 01:44 |
| wallyworld_ | and the repsonseText has the html to render | 01:44 |
| StevenK | It looked like two seperate requests to me | 01:44 |
| wallyworld_ | not if i recall correctly | 01:44 |
| wallyworld_ | but i could be wrong | 01:45 |
| StevenK | In Firebug there was a POST, which returned a 303 See Other, and then a GET which returned 200 OK | 01:45 |
| wallyworld_ | but that's all at the browser level | 01:45 |
| wallyworld_ | i don't think the xhr call sees that | 01:45 |
| wallyworld_ | the xhr call does a post and the browser does it's stuff and ends up giving a repsonse with code = 200 | 01:46 |
| lifeless | StevenK: yes, thats right | 01:46 |
| wallyworld_ | so the xhr call is none the wiser that the redirect has occurred | 01:47 |
| wallyworld_ | so, even if i use an api, that's still an extra round trip | 01:48 |
| wallyworld_ | 1. the api call to do the submission, 2. the call to get the correct url on success | 01:48 |
| wallyworld_ | whereas now it's just the one form submission | 01:49 |
| lifeless | wallyworld_: actually no, an API can be less roundtrips | 01:49 |
| wallyworld_ | it's only one now atm | 01:49 |
| lifeless | wallyworld_: load the form template (moustache) on the initial page load; get the data back from the API (a single POST : reply) and render in client | 01:49 |
| lifeless | wallyworld_: its 2 POST:303 + final url:200 | 01:50 |
| wallyworld_ | it will be a huge job to convert the mp page(s) to mustache | 01:50 |
| wallyworld_ | i was trying to keep it all simple | 01:51 |
| wallyworld_ | ah yes, it is 2. | 01:52 |
| wallyworld_ | i'll have a look after i get this current branch done | 01:52 |
| wallyworld_ | now that i have figured out how factory is screwing me over | 01:52 |
| lifeless | sure, like I said though, we need a consistent story | 01:53 |
| lifeless | what you're doing isn't consistent, nor is it any faster than the API can/should be in principle [modulo headdesk bugs, of course] | 01:54 |
| lifeless | it might be more efficient for porting existing forms | 01:54 |
| wallyworld_ | but it is a much nicer user experience | 01:54 |
| wallyworld_ | if we stay consistent, we keep the crappy ui forever | 01:54 |
| lifeless | no, you misunderstand me | 01:54 |
| lifeless | we have other ajax forms with lovely UI | 01:54 |
| lifeless | that don't do what you have done | 01:55 |
| lifeless | of course we have to break consistency to migrate to something else | 01:55 |
| wallyworld_ | do they need to be backwards compatible though? | 01:55 |
| wallyworld_ | mine does | 01:55 |
| lifeless | well, its not | 01:55 |
| lifeless | (because it breaks non-js browsers) | 01:55 |
| wallyworld_ | no, it works | 01:55 |
| lifeless | so you have all the overhead of server side widget stuff etc | 01:56 |
| wallyworld_ | for non js-browsers, it just submits as normal | 01:56 |
| lifeless | you've got me tied up in knots | 01:56 |
| wallyworld_ | but you don't get the popup confirmation but you do get an informational | 01:56 |
| lifeless | to be functionally equivalent you have to be able to ack the confirmation box | 01:56 |
| lifeless | so you must have added a widget, which you said 'yuck' to before | 01:57 |
| wallyworld_ | here, it's not quite functionally equivalent | 01:57 |
| wallyworld_ | non js just get the informational message | 01:57 |
| wallyworld_ | after form submission | 01:57 |
| wallyworld_ | js get the popup | 01:57 |
| lifeless | ok | 01:57 |
| wgrant | wallyworld_: Doing it the old way (setting document.location) is no more roundtrips. | 02:04 |
| wgrant | wallyworld_: You can tell XHR to not follow the redirect. | 02:04 |
| wgrant | wallyworld_: So you'll get back a 303 with a Location | 02:04 |
| wgrant | Then you set window.location = location | 02:04 |
| wgrant | Done | 02:04 |
| wallyworld_ | ok, cool. didn't know that | 02:04 |
| lifeless | so thats a decent answer here | 02:05 |
| wgrant | Oh, blah, you can't actually block redirects yet. | 02:05 |
| wgrant | So the cheapest may be to say if is_ajax then return a 201 instead. | 02:05 |
| wgrant | It's still evil, but a far, far milder variety. | 02:05 |
| wallyworld_ | can't block redirects? not part of the standard yet? | 02:06 |
| wgrant | Right, it may be in future. | 02:06 |
| wallyworld_ | i did look for how to do that but didn't see anything i could use | 02:07 |
| wgrant | Right. So the view needs to return a 201 like the API, or you need to use the API. | 02:07 |
| wgrant | BugSecrecyEditView already does something similar. | 02:08 |
| wallyworld_ | yes, i wrote the ajax bit | 02:11 |
| wallyworld_ | but it's used as a portlet inside a page | 02:12 |
| StevenK | wgrant: Can haz review? | 02:53 |
| wgrant | StevenK: Why don't we want to clobber yui(2)? | 02:57 |
| wgrant | lifeless: For optimising counts, have we tried doing a "count = SELECT COUNT(*) FROM (real query LIMIT 10000); if count == 10000 then count = 'shitloads';" sort of thing? | 03:18 |
| lifeless | wgrant: we haven't | 03:19 |
| lifeless | wgrant: I can see a few ways that that won't help much | 03:19 |
| lifeless | wgrant: but yeah, sure, that could be a good approach. | 03:20 |
| lifeless | also grr @ bad triage from users that that pump and run | 03:20 |
| wgrant | In some cases it won't help, sure. | 03:20 |
| wgrant | But a lot of common queries just end up being an index walk. | 03:20 |
| StevenK | wgrant: Because it isn't going to change. | 03:25 |
| wgrant | StevenK: Not even with a YUI upgrade? | 03:25 |
| StevenK | wgrant: YUI2 won't change, and we can't upgrade YUI past 3.3 currently | 03:26 |
| StevenK | We can once we stop using yui-deps and related garbage that needs deleting. | 03:26 |
| wgrant | Sure, we can't do it now, but when we want to... | 03:26 |
| StevenK | wgrant: combo-rootdir will likely change again to no longer create a symlink for yui once the icing dependancy dies | 03:27 |
| StevenK | So this isn't the final solution | 03:27 |
| StevenK | And then it can stop being buildout generated. | 03:28 |
| StevenK | wgrant: Ah, thanks. I wasn't sure that I'd convinced you. | 03:40 |
| wgrant | wallyworld_: Sorry, missed the review email before. I've just replied. | 03:42 |
| wgrant | StevenK: Indeed. | 03:42 |
| wallyworld_ | wgrant: np. i'm just about to put up the fix for the mp registration | 03:43 |
| wgrant | Excellent, thanks. | 03:43 |
| StevenK | YAY | 03:43 |
| StevenK | http://www.smh.com.au/environment/weather/weather-settles-after-a-day-of-storms-hail---and-a-tornado-20120214-1t2su.html << Sydney weather is whacked | 03:43 |
| wallyworld_ | it's quite simple in the end | 03:43 |
| wgrant | wallyworld_: The cheap fix I suggested should be about a 10 line diff + tests. | 03:44 |
| wallyworld_ | wgrant: sadly, other model code calls check_permission so i figured it was 'tolerated' | 03:44 |
| wgrant | wallyworld_: There's only a couple of other places. | 03:44 |
| wgrant | PersonRoles makes it pretty cheap to avoid now. | 03:45 |
| wallyworld_ | well, only for those simple checks | 03:45 |
| wgrant | Sure. | 03:45 |
| wallyworld_ | the logic needs to be broken out and the security adaptor call that | 03:45 |
| wgrant | In the complex cases (mostly bugs, branches) it's already delegated to the model. | 03:45 |
| StevenK | And that is going to change anyway? | 03:46 |
| wgrant | Hm? | 03:46 |
| StevenK | We're going to change the complex case of bugs and branches for disclosure? | 03:47 |
| wgrant | Yes, but the precise rules are irrelevant here. | 03:47 |
| wgrant | It's the code structure around them that is under scrutiny. | 03:48 |
| wgrant | Tomboy fails at crash survival :/ | 03:56 |
| wgrant | It mustn't have flushed my notes for about 6 hours yesterday :/ | 03:57 |
| lifeless | less crashing thanks | 03:57 |
| StevenK | wgrant: I have a test here that is doing setupBrowser(auth='Basic mark@ex...:password') I thought you removed that terribleness? | 04:09 |
| StevenK | loltpg | 04:10 |
| wgrant | StevenK: Hm, I suspect that's no longer doing what it thought. | 04:11 |
| wgrant | StevenK: Especially since the domain seems to be ellipsised. | 04:11 |
| wgrant | All the passwords are now 'test' | 04:11 |
| wgrant | Tests still use basic auth. | 04:11 |
| StevenK | wgrant: I'm happy to fix it with a hammer (in a Jeremy Clarkson style) while I'm here. | 04:12 |
| === nhandler_ is now known as nhandler | ||
| StevenK | wgrant: What should it be doing instead? | 04:12 |
| wgrant | StevenK: Which test? | 04:13 |
| StevenK | test_adddownloadfile_nonascii_filename in TestProductFiles | 04:13 |
| StevenK | lib/lp/registry/tests/test_product.py | 04:13 |
| wgrant | So, you can usually just say self.getUserBrowser(user=someone) | 04:14 |
| wgrant | It also takes a url arg. | 04:14 |
| StevenK | Does that work under TestCase, or does it have to be BrowserTestCase? | 04:14 |
| wgrant | TestCaseWithFactory is sufficient. | 04:15 |
| wgrant | BrowserTestCase should probably be eliminated. | 04:15 |
| StevenK | +1 | 04:15 |
| StevenK | wgrant: user= is an IPerson, so I have to reach into IPersonSet to grab mark out? | 04:16 |
| wgrant | Yeah | 04:17 |
| wgrant | Or you can continue using setupBrowser | 04:17 |
| wgrant | But I suspect that test is bad | 04:17 |
| wgrant | Because 'password' is no longer the right password. | 04:17 |
| StevenK | It isn't password, it is test | 04:17 |
| wgrant | Ahhh | 04:18 |
| wgrant | I thought I sedded all of them out :) | 04:18 |
| StevenK | I just thought auth='Basic ...' was sort of evil and needed to be killed | 04:18 |
| wgrant | It is vaguely. | 04:18 |
| wgrant | But it's still used widely. | 04:18 |
| wgrant | I didn't want another 20000 line diff to my name. | 04:19 |
| StevenK | Haha | 04:21 |
| StevenK | Exception-Type: Unauthorized | 04:23 |
| StevenK | Exception-Value: (<zope.browserpage.metaconfigure.SimpleViewClass from /home/steven/launchpad/lp-branches/moar-factory-cs/lib/lp/registry/browser/../templates/productrelease-file-add.pt object at 0x11e60950>, 'browserDefault', 'launchpad.Edit') | 04:23 |
| * StevenK blinks | 04:23 | |
| * StevenK stabs wallyworld_. | 04:55 | |
| wallyworld_ | ouc | 04:55 |
| wallyworld_ | h | 04:55 |
| wallyworld_ | ok, i give in | 04:56 |
| StevenK | wallyworld_: Where is the fix for +register-merge? It's getting quite annoying. | 04:56 |
| wallyworld_ | it's up from review | 04:56 |
| wallyworld_ | for | 04:56 |
| StevenK | I have created a large number of MPs today, though. | 04:56 |
| StevenK | wallyworld_: Do you mind reviewing https://code.launchpad.net/~stevenk/launchpad/moar-factory-cs/+merge/92910 , so I can give wgrant a break? It's nice and small. | 04:57 |
| wallyworld_ | ok | 04:57 |
| wallyworld_ | StevenK: i think you need to pass in some attributes to the favtory method | 05:00 |
| StevenK | wallyworld_: Are you sure, the test passes? :-) | 05:00 |
| wallyworld_ | eg there's one test where sales_system_id is 'foo' and then later 'new' and we need to be sure it's being reset | 05:00 |
| wallyworld_ | an empty test also passes but doesn't test what's required :-) | 05:01 |
| StevenK | wallyworld_: Ah ha. | 05:03 |
| StevenK | wallyworld_: The voucher is redemmed, which ignores whatever is on the CS. Then we remove it, and recreate it. No voucher, so it's new. | 05:03 |
| StevenK | The factory method uses sales_system_id='new' all the time | 05:04 |
| wallyworld_ | yes | 05:04 |
| wallyworld_ | and the test as originally written first used an id of 'foo' | 05:04 |
| StevenK | Yes, and it tested for 'hello' | 05:04 |
| StevenK | So the 'foo' was clearly ignored | 05:05 |
| wallyworld_ | yes, seems so in this case | 05:05 |
| StevenK | wallyworld_: So, fix, or "meh" ? | 05:06 |
| wallyworld_ | perhaps it's ok then | 05:06 |
| wallyworld_ | let me double check | 05:06 |
| StevenK | wallyworld_: Pushing a change, reformating the asserts | 05:07 |
| wallyworld_ | StevenK: i think we can ignore it | 05:07 |
| wallyworld_ | yeah, there's a lot of asserts that are the wrong way around | 05:07 |
| StevenK | You can see one of them in the current diff, line 63 | 05:07 |
| wallyworld_ | and 45 and .... | 05:08 |
| StevenK | Yeah, fixed that too | 05:08 |
| StevenK | assertIs is better | 05:08 |
| * wallyworld_ taps fingers waiting for diff | 05:10 | |
| * StevenK too | 05:10 | |
| wgrant | Huh | 05:11 |
| wgrant | Dynamic bug listings just magically work with a StormRangeFactory. | 05:11 |
| wgrant | I'm shocked. | 05:11 |
| wallyworld_ | StevenK: r=me | 05:11 |
| StevenK | wallyworld_: Thanks! | 05:12 |
| wallyworld_ | np | 05:12 |
| === almaisan-away is now known as al-maisan | ||
| === al-maisan is now known as almaisan-away | ||
| wgrant | Anyone feel like reviewing https://code.launchpad.net/~wgrant/launchpad/fix-debug-timeline-js/+merge/92916? | 06:22 |
| wgrant | -1/+2 | 06:22 |
| wgrant | Ooh | 06:38 |
| StevenK | wgrant: Oooh? | 06:44 |
| wgrant | Flattening access information into bugtaskflat brings COUNT(*) on the 92000 open Ubuntu bugs down from 1100ms to 250ms when hot, because it doesn't have to join against BugSubscription. | 06:45 |
| StevenK | wgrant: I find myself curious why the death of Mochi killed debug_timeline, and why you have href="javascript:void(0)" | 06:46 |
| wgrant | StevenK: removeElementClass doesn't exist, so I assume it was mochi. | 06:46 |
| wgrant | I do that so the link does nothing, because I am too lazy to change it to only create the link when it adds the handler. | 06:47 |
| wgrant | Otherwise it will try to take you to the href :) | 06:47 |
| StevenK | wgrant: r=me, land it | 06:48 |
| wgrant | Thanks. | 06:48 |
| wgrant | I've been using it on DF for a while, so I know it works :) | 06:48 |
| === almaisan-away is now known as al-maisan | ||
| === wgrant changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: - | Firefighting: - | Critical bugtasks: 4*10^2 | ||
| wgrant | lifeless: How significant is the overhead of rabbiting an oops? | 07:02 |
| wgrant | lifeless: I'd basically like to set the soft timeout of some bug searches to 0 when making large changes. | 07:02 |
| StevenK | stub: Thank you for the +1. | 07:06 |
| StevenK | stub: I'll set it to approved, but I need to wait for the previous branch to hit db-devel before landing this. | 07:06 |
| stub | yup. np. | 07:07 |
| === danilo is now known as Guest53490 | ||
| stub | StevenK: mrevell might cry to see entitlements go, but nothing on that is scheduled as far as I know. | 07:24 |
| StevenK | stub: They've never been used, ever. CommercialSubscription handles everything | 07:26 |
| === al-maisan is now known as almaisan-away | ||
| stub | Yer, I was wondering if it was one of those eternal 'we want to use it soon' bits. I'm all for it going. | 07:27 |
| StevenK | If wgrant and I had figured it was this easy to rip out, we may have done it as part of the Doomed 37 | 07:28 |
| wgrant | Curtis confirmed that it can die. | 07:29 |
| StevenK | stub: Working from https://pastebin.canonical.com/60006/ , what do you think of https://pastebin.canonical.com/60117/ as a start? | 07:38 |
| stub | bugnotificationattachment__message__idx is a worry as we should be using that when we delete a message (I suspect we don't delete, just hide at the moment, but that should probably change) | 07:40 |
| stub | bugsummaryjournal.viewed_by - looks like it is missing its foreign key definition? | 07:42 |
| wgrant | StevenK: That may be deliberate. | 07:42 |
| wgrant | Er, stub. | 07:43 |
| StevenK | stub: I'm happy to drop anything we're not 100% sure about and play it safe. | 07:43 |
| wgrant | It may rely on the triggers on BugSubscription to update it. | 07:43 |
| stub | wgrant: yer, just refreshing my memory | 07:44 |
| StevenK | I did ponder adding the unused accesspolicygrant index to that list just to see wgrant be very sad. | 07:45 |
| wgrant | I'll be modifying that schema soonish anyway :) | 07:45 |
| stub | So yes, it shouldn't have a FK constraint. And it can go - looks like we never select on it directly, but only when we already have a subset of rows using product/distro etc. | 07:46 |
| StevenK | stub: Shall I drop bugnotificationattachment__message__idx from the patch? | 07:47 |
| * stub ponders | 07:49 | |
| stub | The table has 0 rows... | 07:50 |
| StevenK | bugnotificationattachment is empty? | 07:50 |
| * StevenK greps | 07:50 | |
| stub | Yup | 07:50 |
| StevenK | COMMENT ON TABLE BugNotificationAttachment IS 'Attachments to be attached to a bug notification.'; | 07:51 |
| StevenK | ... That sounds like bong | 07:51 |
| stub | Maybe some odd 'send patches' use case that will never land. | 07:52 |
| * StevenK blames | 07:52 | |
| stub | So remove the 'DROP INDEX' from your branch. If it goes, it goes because we drop the whole table. | 07:53 |
| stub | I hate the hwdevicedb schema | 07:54 |
| StevenK | Right | 07:54 |
| StevenK | stub: Yes, I'm not amused that 13 are HW indices | 07:55 |
| stub | Same for hwdevicenamevariant - 0 rows. Leave the indexes. If they go, go because we drop the whole table. | 07:56 |
| stub | And hwdmihandle | 07:56 |
| stub | And hwtestanswer | 07:57 |
| stub | all of those actually.... | 07:57 |
| StevenK | hwtest* ? | 07:57 |
| stub | No point removing an index on a table containing 0 rows. No gain, and as we can see helps point to potentially bogus tables | 07:58 |
| StevenK | Leaving bugjob, bugsummaryjournal, hwdmivalue and packagesetinclusion | 07:58 |
| stub | Yes, hwtest* | 07:58 |
| wgrant | I excluded hw* from my purge because I hope to delete them all in one hit within a year :) | 07:58 |
| stub | packagesetinclusion is 0 ows too | 07:58 |
| StevenK | Why is that empty? | 07:59 |
| StevenK | packagesets are widely used | 07:59 |
| stub | Maybe nobody is using nested yet? | 07:59 |
| stub | Maybe we haven't implemented the UI for that? | 07:59 |
| StevenK | stub: Well. There is packagesetinclusion and flatpackagesetinclusion | 08:00 |
| StevenK | Compare and constrant the schema :-) | 08:00 |
| stub | Looks like we neglected to populate packagesetinclusion | 08:01 |
| StevenK | I know flatpackagesetinclusion is used, since I hit it on the derivation work | 08:01 |
| stub | Right. So that will be the exploded version, or should be. | 08:02 |
| stub | So flatpackagesetinclusion contains 118 rows, each of which parent==child. | 08:03 |
| StevenK | Heh, handy | 08:03 |
| stub | So back to my earlier theory that nobody is using nested packagesets | 08:04 |
| StevenK | Right | 08:04 |
| StevenK | I'll dump it then | 08:04 |
| StevenK | So my 17 is down to 3. | 08:04 |
| stub | :-) | 08:04 |
| stub | The revision ones look juicy if that makes you feel bad. | 08:04 |
| StevenK | Oh, which? | 08:04 |
| StevenK | I was ignoring any index that had n_live_tup > 0 | 08:05 |
| stub | revision, revisionauthor, revisioncache | 08:05 |
| stub | Ignoring? The higher that number, the bigger the win | 08:05 |
| StevenK | Oh, crap | 08:06 |
| wgrant | StevenK: Don't dump PSI | 08:06 |
| wgrant | It's just a feature that Ubuntu isn't using yet. | 08:06 |
| StevenK | stub: What does the number mean, then? | 08:06 |
| stub | The number of rows in the table | 08:06 |
| stub | number of live tuples | 08:06 |
| StevenK | Ahh, and the indices listed are utterly unused. | 08:07 |
| StevenK | Right | 08:07 |
| stub | These are all indexes that are unused, yes. | 08:07 |
| stub | So unused indexes on hwsubmissiondevice are a win as it has 100 million rows | 08:07 |
| StevenK | Right. | 08:08 |
| StevenK | stub: I'm concerned sl_log_[12] is in your list. :-) | 08:08 |
| stub | It is? | 08:08 |
| lifeless | wgrant: its proportional to the # of queries | 08:09 |
| StevenK | Line 145 and 146 | 08:09 |
| stub | oh, thought I'd filtered those | 08:09 |
| stub | Forgot 'public schema only' | 08:09 |
| lifeless | wgrant: you can test locally to get a handle on it | 08:09 |
| StevenK | stub: Right, let ignore those 3. Going by largest number first, do you want to get the DB to do the heavy lifting, and ORDER BY n_live_tup LIMIT 15 and I'll hack my patch to drop those? | 08:10 |
| StevenK | stub: If you think we can do more than 15 in a FDT, feel free to bump up the LIMIT | 08:10 |
| stub | StevenK: Why the cleanup push? You trying to get loc credit before the market boom? Have you decided on a beer:loc exchange rate you are going to start selling at? | 08:10 |
| StevenK | stub: No particular reason | 08:11 |
| StevenK | Our DB is large, unwieldy and can be used to scare small children. | 08:11 |
| stub | Dropping indexes should be fast. Should be able to cram a lot of them in. | 08:12 |
| stub | I could time it, but... | 08:12 |
| StevenK | I could time it on DF, but that would be pointless | 08:12 |
| StevenK | Especially since wgrant keeps being evil to DF. | 08:13 |
| StevenK | stub: I thought index deletion scaled with the size of the index? | 08:14 |
| stub | We still need to investigate the targetted ones. eg. LibraryFileContent.sha256. The column should probably be dropped instead as it is WHUI (not sure why it was urgent before) | 08:14 |
| stub | StevenK: rm file_on_disk;sync; | 08:14 |
| StevenK | Because there is a little panic surrounding SHA1 and the librarian uses it quite heavily | 08:15 |
| stub | So we shouldn't drop either the column nor the index, and add the code to the Librarian to populate it on upload. | 08:15 |
| StevenK | I'll let lifeless rule on that. | 08:16 |
| stub | We don't want to drop indexes we will need because that will cause timeouts in the future (because we are unlikely to notice the indexes we need are not there until production). | 08:17 |
| StevenK | Right | 08:17 |
| stub | message__parent__idx unused. The DB model supports threading, but we never display it. UI decision | 08:20 |
| StevenK | stub: I'm paring down your list. Ignoring all indices whose n_live_tup is 3 digits or less and splitting out sl_log*, bug*, lp_*, hw* and specification__date_last_changed__idx, I'm down to 76. | 08:20 |
| * StevenK grep's out the sha256 index, based on previous discussion | 08:21 | |
| StevenK | stub: I'm concerned there are 6 FTI indices implicated | 08:22 |
| * StevenK ignores branch__access_policy__idx too, glancing at wgrant. | 08:22 | |
| stub | StevenK: Right. Probably shouldn't drop the fti indexes, and consider dropping the column (and fti trigger) instead | 08:23 |
| StevenK | stub: Right. | 08:23 |
| StevenK | Hah. sourcepackagepublishinghistory__ancestor__idx | 08:24 |
| StevenK | That one is my fault. | 08:24 |
| stub | wgrant: Have all the no-more-bug-heat-rewriting changes been rolled out? | 08:25 |
| StevenK | stub: I'm down to 68 -- nothing obvious is jumping out, but I can pastebin what I'm down to. | 08:25 |
| wgrant | stub: No, we have to wait 6 days for all the bugs to be updated. | 08:25 |
| wgrant | stub: The new function was deployed last night, and the garbo job updates everything weekly. | 08:25 |
| stub | ok. I want to rebuild the fti indexes, hopefully for the last time. | 08:26 |
| wgrant | bug.fti will hopefully die soon :) | 08:26 |
| wgrant | Although just to be replaced with an equivalent. | 08:26 |
| StevenK | Pity | 08:26 |
| wgrant | Er, not bug.fti, but its index. | 08:26 |
| stub | Have you been investigating tsearch2, or are we finally looking at an external text search engine? | 08:27 |
| wgrant | Nah, still plain old tsearch. | 08:28 |
| wgrant | Just on a flattened table. | 08:28 |
| wgrant | Performs pretty well with GIN and bitmap index combination. | 08:28 |
| * StevenK ignores DSD too, since he isn't sure about its indices at all. | 08:28 | |
| StevenK | stub: https://pastebin.canonical.com/60118/ | 08:31 |
| wgrant | I'm surprised that branch__date_created__idx isn't used | 08:32 |
| wgrant | Likewise the codeimport* ones. | 08:33 |
| StevenK | I'm concerned about karmacache and oauth* | 08:33 |
| wgrant | stub: What about master vs slave? | 08:33 |
| stub | wgrant: We never list 'all branches in order of date created'. And I don't think it will ever be used for ordering when tuples have been selected using some other selection criteria. | 08:34 |
| wgrant | stub: Except that we list the first 5 branches in order of date created. | 08:35 |
| wgrant | https://code.launchpad.net/ bottom middle | 08:35 |
| stub | wgrant: Good call. Should pull the index usage details on the slaves too. | 08:35 |
| stub | wgrant: probably using 'id' instead, since it guarantees order in tests. | 08:35 |
| wgrant | True. | 08:36 |
| stub | Hmm... that is interesting. A lot more unused indexes on _prod_1 than _prod_2. We point all our scripts to _prod_2 IIRC, and prod_1 actually benefits from this lack of load balancing because it will have fewer hot indexes. | 08:45 |
| wgrant | Hmm. | 08:45 |
| stub | StevenK: Ignore indexes that are not also listed in https://pastebin.canonical.com/60120/ https://pastebin.canonical.com/60121/ | 08:45 |
| stub | StevenK: Or I can check it during review | 08:45 |
| StevenK | stub: My current list is https://pastebin.canonical.com/60118/ | 08:47 |
| stub | https://pastebin.canonical.com/60119/ is the reordered master list | 08:48 |
| stub | Will we ever delete a GPG key? I guess not, as even if a user removes it we need to keep it around since there are old published archives signed with it. | 08:50 |
| wgrant | stub: We've done it once AFAIK. | 08:51 |
| wgrant | That may be why those indices exist. | 08:51 |
| czajkowski | aloha | 08:53 |
| stub | StevenK: branch__merge_queue__idx will be needed if merge_queue is ever used. Is that exposed by the API, or is automatic merging a proposed feature in lp? | 08:54 |
| wgrant | Morning czajkowski. | 08:54 |
| czajkowski | wgrant: morning :) | 08:54 |
| adeuring | good morning | 09:00 |
| czajkowski | adeuring: hiya | 09:00 |
| adeuring | hi czajkowski! | 09:00 |
| mrevell | Hi! | 09:01 |
| czajkowski | mrevell: ello | 09:01 |
| === almaisan-away is now known as al-maisan | ||
| StevenK | stub: merge queues are a half-done feature | 09:20 |
| StevenK | stub: I'd like to see them finished, since PQM needs to die a horrible, painful and above all drawn-out death | 09:20 |
| stub | We still can't disable or remove code imports, huh? | 09:28 |
| wgrant | stub: Suspend | 09:29 |
| wgrant | And you delete them by deleting the branch. | 09:29 |
| stub | oauthaccesstoken allows you to specify limitations on an access token, but it isn't used and thus the __product, __project etc. indexes show up | 09:32 |
| stub | I guess that isn't on the radar at the moment | 09:32 |
| stub | Should be be dropping those columns instead? | 09:34 |
| StevenK | stub: Are you happy enough with that patch that I should claim -10 as my very own, put an MP up for the branch and we can nut out the rest in review? | 09:34 |
| stub | oauthaccesstoken + oauthrequesttoken | 09:34 |
| stub | StevenK: Sure | 09:34 |
| wgrant | stub: I don't think the approach they were designed for is adequate. | 09:34 |
| StevenK | stub: I wonder if any of those columns even contain data | 09:34 |
| wgrant | So they should probably go. | 09:34 |
| stub | StevenK: They don't. | 09:34 |
| wgrant | they've certainly never been used. | 09:35 |
| adeuring | wgrant: thanks for the notice on bug 726320 | 09:35 |
| _mup_ | Bug #726320: <type 'exceptions.TypeError'>: add_repository() got multiple values for keyword argument 'src_type' <apport-crash> <i386> <natty> <aptdaemon (Ubuntu):New> < https://launchpad.net/bugs/726320 > | 09:35 |
| StevenK | So perhaps we should not kill the indicies and just drop the columns | 09:35 |
| adeuring | wgrant: do you have already an idea how to dealwith sorting by rank(Bug.fti, ftq('whatever')) | 09:36 |
| wgrant | adeuring: StormRangeFactory is a bit of a challenge due to all the new sort orders. | 09:36 |
| wgrant | adeuring: And that, yeah, I don't know :/ | 09:36 |
| stub | StevenK: Do you want to open a bug or me? | 09:36 |
| StevenK | stub: I'm dealing with allocated.txt and pushing up a branch, so can you? | 09:36 |
| stub | We can drop the indexes now, but we need a bug open for the column removal so that doesn't get lost (well... ) | 09:36 |
| stub | sure | 09:36 |
| wgrant | adeuring: Sorting by milestone, person, reporter names has the same sort of issue. | 09:37 |
| wgrant | adeuring: We need to return more data than usual. | 09:37 |
| stub | Fodder for salgado to reduce his loc impact :-) | 09:37 |
| stub | We should have a tag for that actually... | 09:37 |
| adeuring | wgrant: ah, right, each sort vaule must appear in the result set | 09:37 |
| StevenK | stub: 'here-salgado-salgado' ? | 09:37 |
| adeuring | wgrant: but adding that shoulkd not be difficult | 09:37 |
| wgrant | adeuring: Yeah, it's just a bit messy due to three types of queries that are produced. | 09:38 |
| stub | StevenK: Anyone working on features would find it useful ;) | 09:38 |
| wgrant | adeuring: some of them UNIONs, others wrapped in extra layers to eliminate dupes. | 09:38 |
| wgrant | I'm reworking that stuff atm, which is why I advise not to touch it. | 09:38 |
| wgrant | Hopefully get down to just one or two types of queries. | 09:38 |
| wgrant | And then work out how to get SRF working on top of them. | 09:38 |
| adeuring | wgrant: right -- my basic idea was anyway to use stormrangefactory ;) | 09:39 |
| wgrant | adeuring: My changes make this a bit less important, because scans are about 5x faster in the new schema. | 09:39 |
| wgrant | But I expect to do it afterwards anyway. | 09:39 |
| wgrant | stub: We have a concrete and immediate plan for divorcing the SSO DB, FYI. | 09:41 |
| stub | wgrant: Cool. What is the plan? | 09:41 |
| wgrant | I expect to have it testable by next week. | 09:41 |
| wgrant | stub: SSO becomes an xmlrpc-private client. | 09:41 |
| wgrant | New method on LP takes an OpenID identifier and returns the data SSO needs. | 09:42 |
| stub | wgrant: And what happens when Launchpad is unavailable. Nobody cares? | 09:42 |
| wgrant | stub: SSO just won't return the extra data (Launchpad ID, timezone and teams). | 09:42 |
| wgrant | That appears to be acceptable to ISD. | 09:43 |
| stub | Cool. The HA aspects where what made it a pain and the current system. | 09:43 |
| stub | Typical for HA, when you meet all the goals you end up with a system you don't want to maintain :-) | 09:43 |
| wgrant | stub: Heh. | 09:44 |
| wgrant | Well, LP is far HAer than it used to be :) | 09:45 |
| wgrant | We no longer have ludicrous periods of downtime. | 09:45 |
| stub | If you twist your definition of HA enough, yeah. | 09:45 |
| wgrant | HAer, not HA :) | 09:45 |
| stub | From a traditional POV, we gave the finger to HA and instead opted for controlled scheduled downtime. | 09:46 |
| wgrant | Yeah | 09:46 |
| wgrant | Of reasonable duration, importantly. | 09:46 |
| wgrant | 90 minutes of auth unavailability is clearly unacceptable. | 09:46 |
| wgrant | A minute, which is what we'll probably be at once this is done, seems OK. | 09:46 |
| stub | We currently offer 99% uptime I think (99.65% with fastdowntime, plus unscheduled failures). That figure is passed on to any other systems relying on LP in addition to their own downtime. | 09:48 |
| wgrant | stub: How do you figure 99.65%? | 09:50 |
| wgrant | Outages are currently either 90 or 150 seconds. | 09:50 |
| stub | 5 minutes per day - what we promise rather than what we actually use. | 09:51 |
| wgrant | At 90s that's 99.90% for the day | 09:51 |
| wgrant | Ah | 09:51 |
| wgrant | Right. | 09:51 |
| stub | Need some real metrics for the real answer, including the unscheduled outages :) | 09:52 |
| wgrant | Most unscheduled outages hit SSO too, fortunately :) | 09:52 |
| stub | wgrant: How do we know when the bug heat update has completed? Some garbo output to watch for? | 10:36 |
| wgrant | stub: SELECT COUNT(*) FROM bug WHERE heat_last_updated < '2011-02-13'; | 10:36 |
| wgrant | s/2011/2012/ | 10:36 |
| stub | Cool. I guess we can drop the heat_last_updated soon. That covered by a bug or kanban task or something? | 10:37 |
| wgrant | Might wait for things to settle before doing that. | 10:37 |
| stub | I'll file another tech-debt bug | 10:38 |
| stub | Bug #931987 if you want to claim it | 10:42 |
| czajkowski | ello are uploads not being picked up ? | 11:10 |
| czajkowski | wgrant: sorry not sure where to ask that question in here or -ops | 11:11 |
| wgrant | czajkowski: ops, probably. | 11:11 |
| wgrant | It's possibly related the pgbouncer issue. | 11:11 |
| czajkowski | k thank | 11:11 |
| tumbleweed | wgrant: ta | 11:13 |
| czajkowski | tumbleweed: yo're here | 11:13 |
| tumbleweed | I haven't gc-ed channels in a while... | 11:14 |
| === Guest53490 is now known as danilos | ||
| === matsubara-afk is now known as matsubara | ||
| rick_h | morning | 12:13 |
| czajkowski | rick_h: aloha | 12:18 |
| StevenK | rick_h: I note your lack of reply. However, I fixed your combo-rootdir issue in devel. | 12:27 |
| rick_h | StevenK: lack of reply? | 12:28 |
| rick_h | StevenK: I saw that MP, will pull that and try it out thanks! | 12:28 |
| StevenK | rick_h: You said you'd reply to my combo-url mail on the list. | 12:30 |
| StevenK | rick_h: Pull it out? Just merge devel? :-) | 12:30 |
| rick_h | StevenK: ah, yea well my tests came back failing last night | 12:30 |
| rick_h | part of my reply was going to be an update on the new test templates and such | 12:30 |
| StevenK | Ah, kay | 12:31 |
| rick_h | I got a bunch of js timeouts so both failed and I'm working on figuring out wtf this morning | 12:31 |
| rick_h | no landing from me :/ | 12:31 |
| StevenK | rick_h: The AJAX log branch is seperate, I'm guessing? | 12:31 |
| rick_h | yea, but that new test came back with js timeout | 12:31 |
| StevenK | Strange | 12:31 |
| rick_h | so something bigger perhaps? or maybe I just caught a bad ec2 day | 12:32 |
| StevenK | I threw two branches at ec2 today, both passed nicely. | 12:32 |
| rick_h | *sigh* well, I'm peeking and will try to relaunch in a bit | 12:32 |
| rick_h | but yea, so I didn't reply to the email yet | 12:33 |
| rick_h | I hate it, tests all pass locally peachy, only way to "check" is to toss things at hours of ec2 and cross your fingers | 12:34 |
| StevenK | Welcome to Launchpad. :-/ | 12:34 |
| deryck | adeuring, rick_h -- https://plus.google.com/hangouts/extras/talk.google.com/orange-standup | 14:32 |
| === Ursinha_ is now known as Guest56696 | ||
| sinzui | jcsackett, Do you have time to review https://code.launchpad.net/~sinzui/launchpad/listify-cached-licences/+merge/92893 | 15:00 |
| jcsackett | sinzui: good timing, i have another branch i need to review this morning anyway, so certainly. :-) | 15:01 |
| jcsackett | looking at yours now. | 15:01 |
| sinzui | jcsackett, fab, I am looking at your first branch now | 15:01 |
| sinzui | jcsackett, replied with a patch | 15:19 |
| jcsackett | sinzui: i have replied to yours as well, with some notes/nitpicks. r=me. | 15:23 |
| jcsackett | sinzui: i will patch mine, thanks for the code. wasn't sure how to test just registering, nor did i think it was strictly necessary. happy to add it though. :-) | 15:23 |
| rick_h | hmm, so if I fire up an ec2 test run and ssh to the box I should be able to find the files and manually fire off test commands while it runs right? | 15:27 |
| === al-maisan is now known as almaisan-away | ||
| === salgado is now known as salgado-lunch | ||
| rick_h | looks like no, I can't boooo | 15:54 |
| rick_h | deryck: ok, big duh...the 8 tests that fail are the 8 updated ones to look in the build dir | 16:10 |
| deryck | rick_h, aren't they all being updated to look in the build dir? or just in this branch it's only these 8 that are changed? | 16:11 |
| rick_h | deryck: only those 8 are changed. I'm trying ot see if hte build dir is done via the test runner | 16:12 |
| rick_h | make is run | 16:12 |
| rick_h | which should make the build dir | 16:12 |
| deryck | rick_h, yeah, that's what I was saying this morning; it smells like the files aren't there. | 16:13 |
| rick_h | deryck: yea, the make command is in run_demo_server so wonder if the YUI js tests are run in a different place and the build dir is gone/not there yet | 16:14 |
| rick_h | deryck: so yea, guessing the issue is how the ec2 test script is run. Working on getting my head around it | 16:14 |
| rick_h | but that makes sense now | 16:14 |
| === matsubara is now known as matsubara-lunch | ||
| === salgado-lunch is now known as salgado | ||
| === matsubara-lunch is now known as matsubara | ||
| lifeless | inline adverts in the twitter stream. Disappointing twitter, disappointing. | 17:37 |
| === _mup__ is now known as _mup_ | ||
| === maxb is now known as Guest79372 | ||
| czajkowski | abentley: are you about for a quick question, not urgent , can ping tomorrow it's re https://answers.launchpad.net/launchpad/+question/184925 | 18:01 |
| === nhandler_ is now known as nhandler | ||
| benji | when doing a make in a fresh devel checkout I get "ImportError: No module named convoy.meta"; I followed the instructions in Steve's "Convoy is ready for others | 18:32 |
| benji | " email, so I'm wondering what step I left out | 18:32 |
| rick_h | benji: convoy should be installed via an apt-get upgrade | 18:33 |
| rick_h | it's part of the ppa | 18:33 |
| benji | rick_h: I would have thought the same thing | 18:34 |
| lifeless | rick_h: what do we import convoy for ? | 18:34 |
| lifeless | benji: what flaour of Ubuntu are you running? | 18:34 |
| benji | oh; I wonder if it's not packaged me | 18:34 |
| rick_h | lifeless: convoy.meta helps walk the list of YUI modules we defined | 18:34 |
| rick_h | so thatit knows how to import it | 18:34 |
| benji | lifeless: an older one ;) | 18:34 |
| benji | let me look exactly | 18:34 |
| lifeless | benji: tsk! | 18:34 |
| benji | lifeless: I have a newer VM, let me fire it up and see if it works there | 18:35 |
| rick_h | benji: ah, ok that makes sense then | 18:35 |
| lifeless | benji: you know we're meant to upgrade @ beta release each ubuntu release ? :) | 18:35 |
| benji | lifeless: I mostly do, I forgot which VM I was running | 18:36 |
| rick_h | lifeless: do things run well on precise now? | 18:36 |
| rick_h | I've been waiting for all that pgsql stuff to settle before upgrading the laptop, but looking forward to | 18:36 |
| === deryck is now known as deryck[lunch] | ||
| lifeless | rick_h: so what I encourage folk to do, and I do myself, is run my host in $ubuntu-current (usually upgrading around alpha 1), and do dev in an LXC lucid container | 18:37 |
| rick_h | I need to play with lxc I suppose. I've not messed with it at all yet | 18:37 |
| benji | rick_h: it's pretty cool, if a bit young still | 18:39 |
| benji | rick_h: yay, my newer VM works fine | 18:55 |
| abentley | czajkowski: I wish that were a quick question, but I've looked at it several times and not come up with an answer. | 18:55 |
| abentley | czajkowski: It looks like it's failing to gpg-sign a revision. | 18:55 |
| abentley | czajkowski: I don't know why it's failing, but I don't think it should be trying to sign in the first place. | 18:56 |
| abentley | lifeless: this is a CVS import. We don't gpg-sign those, do we? https://code.launchpad.net/~rpm5/rpm/trunk | 18:57 |
| lifeless | I don't think we sign any imports | 18:57 |
| lifeless | IMBW | 18:57 |
| lifeless | but if we sign any, I expect we sign all | 18:57 |
| rick_h | benji: awesome | 18:58 |
| czajkowski | abentley: ok just confused as I've never seen that kind of thing before again apologise for the question for the coming weeks | 19:00 |
| lifeless | abentley: actually IIRC we did setup signing of imports way back with baz, it was a religion then | 19:01 |
| lifeless | abentley: so I expect our cscvs infrastructure does indeed sign things | 19:01 |
| abentley | lifeless: Maybe I'll pull the successful imports and see if they're signed. | 19:01 |
| abentley | lifeless: 10254 commits not signed | 19:06 |
| lifeless | abentley: from a cvs import ? | 19:07 |
| lifeless | svn ones are all bzr-svn now, which won't sign as we're not invoking the commit codepath; ditto hg and git. | 19:07 |
| abentley | lifeless: yes, https://code.launchpad.net/~rpm5/rpm/trunk. 62 commits were signed (but with keys my box doesn't know) | 19:07 |
| lifeless | abentley: verra interesting | 19:08 |
| lifeless | abentley: I wonder if the different import slaves have different configs for bzr or something | 19:08 |
| abentley | lifeless: Yes, I wonder too. | 19:08 |
| abentley | lifeless: Also wonder how many of our CVS imports are failing. | 19:09 |
| === deryck[lunch] is now known as deryck | ||
| === matsubara_ is now known as matsubara-afk | ||
| === mbarnett` is now known as mbarnett | ||
| lifeless | whats the easiest way to get a js repl ? | 21:33 |
| benji | lifeless: FF Ctrl-shift-K | 21:37 |
| lifeless | benji: ok, and thats bare in a new tab.... | 21:38 |
| lifeless | benji: so I should be less vague; I want to play with handlebars.js | 21:38 |
| lifeless | ah, http://tryhandlebarsjs.com/ looks the ticket | 21:40 |
| benji | lifeless: hmm, maybe find a page that already loads it? Or do some document.createElement("script"); | 21:41 |
| benji | or that :) | 21:41 |
| lifeless | abentley: had you seen http://engineering.linkedin.com/frontend/client-side-templating-throwdown-mustache-handlebars-dustjs-and-more in evaluating moustache ? [just curious] | 21:41 |
| abentley | lifeless: no. Thanks for pointing it out. | 21:42 |
| === salgado is now known as salgado-afk | ||
| abentley | lifeless: "...none of the templating options worked well across client and server unless the server could also execute JavaScript..." | 21:47 |
| lifeless | its an interesting statement, but missing details needed to evaluate it | 21:49 |
| lifeless | ... which is a bit disappointing | 21:51 |
| abentley | lifeless: Agreed. | 21:51 |
| abentley | lifeless: I don't agree that Mustache has "clean" syntax. Am I silly for wanting loops to be distinct from conditionals? | 21:52 |
| lifeless | mmm, I could go either way | 21:53 |
| lifeless | its very pithy as it stands | 21:53 |
| rick_h | lifeless: just create an index.html with handlebars in it. Then just use things like the normal dev tools if you want, but I find just using livereload and that test.html file enough to tinker away | 21:57 |
| rick_h | lifeless: https://github.com/mockko/livereload/blob/master/README-old.md | 21:58 |
| rick_h | lifeless: another cool js toy area http://jsfiddle.net/ | 22:00 |
| rick_h | lifeless: that's cool because you can share a fiddle via link to others, interactive pastebin | 22:00 |
| wgrant | jelmer: Hi, you have a couple of bits of QA. | 22:12 |
| rick_h | StevenK: ping, you see my email to the list today? | 22:36 |
| StevenK | Wha? E-mail? | 22:41 |
| StevenK | rick_h: I've been on mumle since I started, e-mail is a secondary concern | 22:42 |
| StevenK | Bah. mumble | 22:42 |
| StevenK | rick_h: make build can't run combobuild | 22:46 |
| StevenK | If you just run make (Which is the 'inplace' target), that will run combobuild | 22:47 |
| rick_h | StevenK: ok, so what's the "right" way for me to get combobuild run in tests? | 22:54 |
| rick_h | StevenK: my issue is I can't ec2 test/land anything because make check only runs make build and combobuild never gets run | 22:55 |
| rick_h | StevenK: if I break out combobuild to be two parts, one that just copies files into build/js/lp and one that uses convoy to generate meta.js can build run combobuild? (the first part) | 22:55 |
| StevenK | rick_h: Yes. I can do that for you today. | 22:56 |
| rick_h | StevenK: ok, if you've got time. I thought I couldn't just run combobuild form make build, but couldn't recall why | 22:57 |
| rick_h | but yea, all the JS tests now point to the build/js/... for their test files so I need that to be populated for make check | 22:57 |
| StevenK | rick_h: Because convoy won't be installed everywhere, and machines like banana and nutmeg use make build | 22:58 |
| StevenK | And gandwana for that matter. I should fix that. | 22:58 |
| rick_h | StevenK: ok, I wasn't sure 100% | 22:58 |
| rick_h | the other thing would be to break out the meta generation stuff of convoy out but think that'd be making for more problems | 22:58 |
| StevenK | Right | 22:59 |
| StevenK | I don't think there is any gain there | 22:59 |
| rick_h | my only concern with the splitting of meta.js generation is that when we run that, we only want to generate the meta.js for the non-minified files | 23:00 |
| rick_h | so I think what we do now is, copy files, generate meta.js, minify files | 23:01 |
| rick_h | so it may be a pita to have to break that into 3 steps ugh | 23:01 |
| StevenK | You think? :-) | 23:02 |
| StevenK | I'm happy with 2 steps | 23:02 |
| rick_h | well I know convoy only looks at the root directory and generates metadata for all files in there | 23:02 |
| rick_h | if we say copy/min files in one step, and generate meta.js with convoy in another | 23:03 |
| rick_h | it'll have dupe metadata info | 23:03 |
| StevenK | Right | 23:04 |
| StevenK | Perhaps convoy needs a patch to ignore minified files | 23:04 |
| rick_h | yea, maybe an --exclude regex or something | 23:04 |
| rick_h | nvm, already has an exclude regex for the cmd line version | 23:05 |
| StevenK | So we can --exclude '-min.js' ? | 23:05 |
| rick_h | well except I don't think we call it from the cmd line do we? We import extra_metadata? | 23:06 |
| StevenK | utilities/js-deps -n LP_MODULES -s $BUILD_DIR/lp -o $BUILD_DIR/lp/meta.js >/dev/null | 23:06 |
| StevenK | That is called from combo-rootdir | 23:07 |
| rick_h | right, js-deps so that's got to be updated to pass the args to main() or whatever | 23:07 |
| StevenK | What? | 23:07 |
| StevenK | It already handles this | 23:07 |
| StevenK | Run utilities/js-deps --help | 23:07 |
| rick_h | yea, sorry, main() catches it | 23:08 |
| StevenK | You're trying to solve problems that don't need solving :-) | 23:08 |
| rick_h | sorry, thoguht it was handled outside of the main we imported | 23:08 |
| * rick_h is taking a second to trace code :) | 23:08 | |
| rick_h | ok, well anyway. Sounds like a plan to me. We can keep it two steps if we can make sure it skips the -min.js files | 23:08 |
| wgrant | Bah | 23:20 |
| wgrant | Only got Product:+index 99% down by 25%. | 23:21 |
| wgrant | Although mean down by 45%, which I guess is better. | 23:23 |
| lifeless | so, mean means all users notice it | 23:29 |
| lifeless | 99% means outliers notice it | 23:29 |
| lifeless | if both shift by the same amount, and 99% isn't capped by timeouts, then you removed a constant overhead | 23:29 |
| wgrant | I know. | 23:29 |
| lifeless | :P | 23:30 |
| wgrant | Ah, there we go, the remaining major win is trivial. | 23:32 |
| * wgrant fixes. | 23:32 | |
| wgrant | lifeless: Do you know what the current stats targets are on prod? | 23:41 |
| wgrant | That query with massive planning overhead that I found last week is still only 50ms on DF cold, and 10ms hot. | 23:41 |
| wgrant | Including planner. | 23:41 |
| lifeless | I can find out if you need | 23:42 |
| jelmer | wgrant: yes, sorry - it's first on my list of things to do tomorrow | 23:42 |
| lifeless | should we roll it back in the interim ? | 23:43 |
| jelmer | lifeless: roll back my change you mean? it's only just landed | 23:44 |
| wgrant | It landed a couple of days ago | 23:44 |
| wgrant | That's two or three LP deployments :) | 23:44 |
| lifeless | jelmer: it's 10 commits back, landed on the 13, its now the 15th, adjusting for tz's its been in trunk for 36 hours | 23:47 |
| lifeless | jelmer: anything in trunk is a pipeline stall for the whole team, QA on it *has* to happen promptly. | 23:47 |
| lifeless | ideally the person landing, but failing that anyone with knowledge - and yes, that makes things with specialist knowledge benefit from explicit handoffs for QA. | 23:48 |
| lifeless | jelmer: if you can tell us what you were going to do, we may be able to do it | 23:48 |
| jelmer | lifeless: it only landed on qastaging 21 hours ago according the qa tagger | 23:48 |
| lifeless | even so, thats plenty of time to have either qa'd or handed off the qa | 23:49 |
| wgrant | lifeless: What's the process for live index creation these days? | 23:51 |
| wgrant | Apply before or after landing? | 23:51 |
| wgrant | It should take about 60ms to create :) | 23:51 |
| lifeless | wgrant: land on qastaging, add and QA there, then prod. | 23:52 |
| wgrant | land on devel, add and QA on qastaging, then prod? | 23:53 |
| lifeless | yes | 23:53 |
| wgrant | It turns out that ProductReleaseFile.productrelease is unindexed. | 23:53 |
| wgrant | So I guess the table is just pathetically small. | 23:53 |
| lifeless | 74K rows | 23:54 |
| wgrant | Oh, right, the PRF. | 23:54 |
| wgrant | p-r-f, sorry | 23:54 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!