[00:46] Bleh, webservice.patch is giving me 401 :-( [01:08] How am getting ValueError: (, 'subscribe', 'launchpad.Edit'), the webservice is for the owner of the bug :-( [01:10] StevenK: The owner doesn't always have access if it's private [01:11] Which it isn't. [01:14] wgrant: Diff is at http://pastebin.ubuntu.com/1195794/ , test output at http://pastebin.ubuntu.com/1195796/ [01:16] StevenK: Have you checked that webservice_for is not defaulting to an RO token? [01:17] or anonymous ? [01:17] It's not anonymous, since a person is being passed in [01:19] Oh, bleh, I bet that's it. [01:20] permission=OAuthPermission.READ_PUBLIC [01:20] Yeah [01:20] :D [01:35] lifeless: You'll never guess what the fix is. [01:39] Is it just a matter of checking in the subscriber if the modification event actually has any modifications? [01:40] StevenK: don't use the API ? [01:40] if event.edited_fields is None: return [01:46] wallyworld, wgrant: https://code.launchpad.net/~stevenk/launchpad/subscription-no-longer-updates/+merge/123467 [01:46] StevenK: busy, will look soon [02:09] wgrant: Do you think 1021773 and 1035338 could be dupes? I can not load the oops to compare them. [02:11] StevenK: Yes, they're probably dupes [02:12] And they're probably related to a lot of the scanner timeouts too [02:12] Some thing or things are holding locks on branch for many seconds [02:13] StevenK: What if edited_fields is an empty list or so? [02:14] We probably just care if it evaluates to boolean false [02:15] wgrant: I can change that bit [02:17] wgrant: http://pastebin.ubuntu.com/1195864/ Is that your only concern? [02:20] StevenK: I think so [02:20] StevenK: Are there similar subscribers on other objects? [02:34] wgrant: I have no idea -- I didn't think anything but bugs used Zope subscribers. [02:34] And I didn't know bugs used them until Friday afternoon. [02:34] It's largely localised to Bugs, but I'd look around just in case [02:34] bugs [02:34] answers [02:34] soyuz has some bits I think [02:34] code [02:35] localised, no, centre of gravity, yes. [02:35] Soyuz doesn't really use them [02:35] Certainly not for ObjectModifiedEvents [02:35] Soyuz has one. [02:35] Oh? [02:35] In other news, where are my OOPS reports [02:35] for="lp.soyuz.interfaces.archivesubscriber.IArchiveSubscriber [02:35] lazr.lifecycle.interfaces.IObjectCreatedEvent" [02:35] handler="lp.soyuz.mail.notifications.notify_new_ppa_subscription"/> [02:35] Ah [02:37] wgrant: I can't think of a similar pattern off the top of my head, anyway. We call a method on X, don't modify X and return Y [02:37] I'd look around for the other ObjectModifiedEvent subscribers and see what they do [02:38] 24460 0 Archive:EntryResource:getPublishedBinaries [02:38] ouch [02:40] not deployed yet ? [02:42] A couple of hours ago [02:42] So tomorrow's report should be clean [02:42] hopefully [02:46] wgrant: Everything looks fine -- I'm not sure if subscribing to a branch over the API causes the same issue. [02:57] wgrant: Which is immune, so everything is fine. [02:59] OK [02:59] r=me, thanks [04:24] review plox: https://code.launchpad.net/~lifeless/js-oopsd/post/+merge/123474 [04:25] wallyworld: or StevenK: ^ wgrant, is obsessing on performance. [04:26] Correctly obsessing; this missing index is at the root of all but one known disclosure performance regression :) [04:26] otp, just a sec [04:36] wgrant: what missing index is that? [04:38] * StevenK stabs bugactivity [04:38] wallyworld: the inverse index on a M:M join table [04:39] wallyworld: See #launchpad-ops. accesspolicygrant(grantee) isn't indexed, so finding a list of policies that a person can see uses accesspolicygrant(policy, grantee) [04:39] which M:M table? [04:39] And takes 100ms rather than 1ms [04:39] ok [04:39] This fixes all the branch access checks [04:39] excellent [04:39] It possibly won't fix code.launchpad.net/ubuntu, as the MP query there is pretty obscene, but I have a simple query workaround for that [05:09] Sigh, it's been so long since I've had do preloading, I've forgotten what to look for. :-( [05:11] * lifeless begs for a review [05:14] lifeless: in your overview, you say any browser to do "POST" or "GET", but in your "Allow-Methods" header, you have "PUT", "DELETE", am I missing something there? [05:18] and your test seems a little bit circular (rather than asserting what the actual content is, it asserts that the content matches the list that you say it should be) [05:18] so if I put whatever I want it cors_headers, the test still passes. [05:18] (vs saying "GET" is in the list of Access-Control-Allow-Methods) [05:19] so it depends what you want to assert [05:19] thats true [05:19] if you are asserting that passing the cors_headers is hooked up == good [05:19] I had a more specific test but perhaps over enthusiastically refactored [05:19] asserting that it conforms to the spec == better. [05:19] the it should be GET+POST [05:20] jam: I'll a) fix the static set of headers and b) convert the preflight check to be manual rather than circular. [05:21] lifeless: aside from that sanity check, I don't know that it is worth having *me* fully understand CORS, so I'll trust that we can iterate until everything works. [05:21] (If it doesn't work properly in browsers, then we update the test suite until it does) [05:22] I'm not sure anyone really understands CORS. [05:22] I've not read a more confusing spec. [05:22] lifeless: which is why I think iterating towards 'working' is good. [05:22] As browsers may not conform at all to the spec either. [05:22] that too [05:22] confusing specs tend not to be followed by anyone very well :) [05:27] lifeless: review posted [05:28] jam: I've pushed a revised version [05:28] jam: if there was nothing more in the review, it should be good [05:29] lifeless: the only other thing I thought about, do we want to have a wide-open access policy? === Ursinha is now known as Ursinha-afk [05:30] also need a review for https://code.launchpad.net/~lifeless/python-oops-tools/bug-1048470/+merge/123480 please :) [05:30] jam: yes. [05:30] lifeless: mostly just making sure that was true. It might be good to have a code-level comment about it. [05:30] jam: Folk that want to attack us can just ignore the policy entirely. [05:30] as it looks like a security hole [05:30] I thought it is probably ok [05:30] jam: we can't ever trust the data we get for this service at all. [05:31] lifeless: aiui, this is about the fact that launchpad.net may send out a javascript header, but we want to post the OOPS to oops.canonical.com, right? [05:31] and anyone can just post to oops.canonical.com [05:32] and this is just about telling javascript that it is ok that even though launchpad.net gave you the JS file, it can send data to oops. [05:32] yes, or u1, or ... [05:33] lifeless: so you've gotten an approve* from me [05:33] tight rules are used to prevent query methods that return sensitive data, and are secured by cookies or whatever, from being fooled via CRSF attacks and the like [05:33] not sure if you need someone official on it :) [05:33] jam: you shold be official [05:34] wallyworld: thanks! I totally don't know why I didn't thinkyou'd reviewed. [05:34] wallyworld: slow mail day or something. [05:35] np [05:35] lifeless: I've never been officially given the 'launchpad reviewer' title, at least that I can remember. [05:36] jam: you're in the group; do you have a mentor reviewer to give you the LP magic sauce ? [05:37] TAL, please die in a large chemical fire. http://pastebin.ubuntu.com/1196061/ [05:38] wallyworld: if I can trouble you again .. https://code.launchpad.net/~lifeless/python-oops-tools/bug-1048470/+merge/123480 [05:38] lifeless: https://code.launchpad.net/~lifeless/python-oops-tools/bug-1048470/+merge/123480 just to validate assumptions, the decoding of 'data' already maps things to Unicode, so that we don't have to worry that the data might be a UTF8 string that we are then doing .encode('utf8') on (which will break with the magic decode step). [05:38] wallyworld: its about 4 lines of code [05:38] sure [05:38] wallyworld: or... jam has it :P [05:38] wallyworld: so no worries [05:38] jam: right, its coming out of the ORM [05:38] jam: so its always unicode. [05:38] jam: its assigned slightly higher in the file [05:38] lifeless: so the thing you changed was 'pageid' related, but the test is testing 'topic'. [05:39] jam: topic in the wire protocol is mapped to pageid in the schema [05:39] the schema is old and rather more LP centric than the wire protocol. [05:39] creaky even [05:39] lifeless: I don't think I've ever had an official mentor for obtaining LP secret sauce. [05:40] jam: so, lets get you one. I know, gmb, he has nothing to do :P [05:40] jam: you clearly know python and how to do code review; the only thing you need is to convince a mentor reviewer than you can tell the difference between code you grok and code that will contain surprising gems. [05:41] lets see what tarmac thinks of that one [05:42] lifeless: should there also be a test for the utf8 encoded oopsid? [05:47] wgrant: http://pastebin.ubuntu.com/1196061/ [05:47] wallyworld: mmm, I only changed that because the test I wrote caught it :) [05:47] wallyworld: so, I don't think an explicit one is called for, no. This whole code could be radically improved by using a template engine. [05:47] ah, ok. gotta love tests :-) [05:47] +1 to the template engine [05:47] yeah, I added the test to validate my assumption about the cause of the problem, and then fixed everything it uncovered. [05:47] Stuff trying to understand this stuff :> [05:48] StevenK: It's probably raising an AttributeError because your code is terrible [05:48] Anywhere inside the attribute it complains about [05:48] An AttributeError will just be squashed into that useless LocationError [05:49] wgrant: I didn't think http://pastebin.ubuntu.com/1196075/ was terrible :-( [05:50] StevenK: Possibly personID isn't on IBugActivity [05:50] So it's not accessible [05:52] ForbiddenAttribute: ('personID', ) [05:52] But it's there if you dropkick the security proxy [05:53] that would be the attribute not being on the interface [05:54] It's SQLBase, so I have no idea why it isn't. [05:54] because they aren't magic. [05:55] They have to be explicitly listed like all other attributes. [06:08] The interface is not SQLBase [06:08] SQLBase only affects the model [06:08] Yeah, I realized after lifeless' comment [06:09] wallyworld, wgrant: https://code.launchpad.net/~stevenk/launchpad/bugtask-activity-preload/+merge/123484 [06:10] StevenK: Do you really care about just ValidPersonCache? [06:10] You probably want the Person too [06:10] Which means you want PersonSet.getPrecachedPersonsFromIDs [06:11] wgrant: It's strange, if I did load_related(Person, ...) the query count went to 15. [06:22] wallyworld: I was able to reproduce the request a review bug on prod a couple of hours ago [06:22] wallyworld: It works fine until you click to submit [06:22] Then it just disappears and does nothing [06:22] wgrant: worked for me locally [06:22] i didn't hit submit on prod, i'll try on qas [06:22] wallyworld: With the yui 3.5 FF set? [06:23] i think so, unless i've remade my schema, which i may have done [06:23] the bug said the link wasn't even clickable [06:23] which is not the case anymore [06:24] It doesn't say that the link was unclickable [06:24] It even says "I was trying to request lifeless review a db patch branch.", which suggests it died post-input [06:24] it says the console shows an error when it is clicked [06:24] which implies the link didn't work [06:24] It doesn't say anything about clicking, just that the green link was being used [06:25] how do you ue a link without clicking it? [06:25] You don't, but it doesn't say it failed during the clicking of the link [06:25] Just that it failed some time while using the functionality behind that link [06:26] wgrant: http://pastebin.ubuntu.com/1196108/ when preloading Person only [06:26] sure, but i seem to have a recollection i saw the same issue and the link itself didn;t work [06:26] wallyworld: Hm, I've never seen that [06:26] StevenK: But it does reduce the query count somewhat? [06:27] wgrant: It does not. It goes from 13 to 15. [06:27] Oh [06:27] I wonder if you're failing to invalidate the cache properly [06:27] 97+ Store.of(bug).flush() [06:27] ^ Not sufficent? [06:27] flush is not invalidate [06:27] flush flushes [06:27] invalidate invalidates :) [06:29] wgrant: submitting a review request works fine on qas too [06:30] wallyworld: Ah, qastaging only has 3.5.1 enabled for rick [06:31] :-( i assumed it was on for everyone in lp [06:31] Likewise, but I just checked and it's not :( [06:32] bollocks [06:32] Heh, we just passed MP 123456 a few hours ago [06:33] wallyworld: Try rerequesting a review from lifeless on https://code.launchpad.net/~wgrant/launchpad/i-am-stupid/+merge/123478 [06:34] yep, so that failed [06:37] wgrant: So my numbers were off. Slightly [06:37] wgrant: Putting in list(getUtility(IPersonSet).getPrecachedPersonsFromIDs( [06:37] ids, need_validity=True)) [06:38] Still has a bunch of queries against ValidPersonCache [06:38] That's very odd [06:38] The same number of queries? [06:39] So with no preloading, 50 bugactivity rows is 109 queries. With that getPrecachedPersonsFromIDs, it drops to 33. [06:39] Oh, 50? [06:39] You can't test with 50 [06:39] The dev storm cache size is only like 100 [06:40] Why not? [06:40] Also, that's going to be slow even if the cache is huge :) [06:40] No point going above, say, 5 [06:40] My original test had 10 [06:42] 10 bugactivity rows drops from 29 queries to 7. [06:42] That sounds roughly correct, right? [06:42] Yeah [06:43] wgrant: Well, 5 rows is 7 queries, as is 10. Which is good. [06:43] Yes :) [06:43] wgrant: http://pastebin.ubuntu.com/1196125/ [06:44] StevenK: I'd inline the list comp, but otherwise good [06:44] Ah yes, that would work. [06:46] Haha, wallyworld approved it 32 minutes ago, I just didn't notice. [06:46] Heh, so he did [06:47] I was right, though :) [06:47] Not disputing that one bit. [07:40] good morning [07:50] wgrant: Heh, if not event.edited_fields is not quite the solution -- the 313 failures from ec2 attests to that. === almaisan-away is now known as al-maisan === al-maisan is now known as almaisan-away [08:28] stub: Morning === almaisan-away is now known as al-maisan [09:44] stub: Could you review https://code.launchpad.net/~wgrant/launchpad/i-am-stupid/+merge/123478? [09:44] wgrant: love the name [09:45] I try. [09:45] such a let down, just missing indicies [09:45] lol [09:45] Shouldn't it go into db-devel or something? [09:46] Nah, nowadays indices go to devel and get applied live, without downtime [09:46] Ooooh. Exciting :D [09:47] k [09:48] wgrant: Not stupid, I was hoping the primary key indexes would make these unnecessary [09:49] The only really important one is apg(grantee, policy), and it was pretty obvious that that was necessary from the start, but I apparently forgot to create it [09:49] well... grantor is needed or person merge explodes [09:49] yeah, but the (policy, grantee) would often be usable instead of the inverse [09:50] These can go live now? [09:50] Please [09:50] I will be watching the DBR clear up immediately :) [09:51] Well [09:51] I would be if my Yubikey wasn't flashing angrily at me [09:51] hah [09:53] This is inconvenient [09:54] usually I have to go find my phone and hope it's charged [09:54] wgrant: Might be worth making the compound indexes UNIQUE? [09:54] wgrant: Don't think it makes any difference now, but a future planner might notice. [09:54] stub: Right, it doesn't make any difference now. [09:54] But I could... [09:54] Possibly a good idea [09:54] yeah [09:57] * wgrant does so [09:59] stub: That's done [09:59] Diff not updated yet, though [10:01] Hm [10:01] Do we still call unique indices __key? [10:01] Rather than __idx? [10:04] yeah, __key [10:07] stub: Done [10:07] I think that's it... [10:09] mgz: your summon worked! [10:09] magic :) [10:14] wgrant: All done on production [10:14] and qastaging [10:16] stub: Thanks [10:17] branch and APG will hopefully drop off the top of the DBR's Most Read Tables list... [10:29] Well, APG is dropping to nothing, but branch is still terrible :( [11:58] stub: So, I see 9.2 has been released... just as we are almost completely migrated to 9.1 :) [11:59] chaining repliate ftw! === gary_poster|away is now known as gary_poster === benji changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: benji | Firefighting: - | Critical bugs: ∞ [12:59] benji: poke about lpsetup. It appears that my patch cannot land because the tarmac process is broken? [12:59] (someone said that if a patch breaks, it doesn't teardown the lxc instance, so the next submission will fail because the lxc is still there) [13:00] I'm not entirely sure who to talk to, but I figured starting with someone on yellow might be reasonable. [13:00] jam: hmm, I hadn't heard of that; I'll see what I can do about it [13:01] benji: https://code.launchpad.net/~jameinel/lpsetup/missing_lxc_1045728/+merge/122663 is the merge request [13:01] bac: are you aware of such a problem? ^^^ [13:01] the failure from tarmac was 'juju', 'bootstrap', '-e', 'lpsetup-testing-lxc' was failing. [13:04] benji, jam: the first thing to do is to ssh into the machine and have a look around. [13:04] you should be able to 'ssh tarmac@lptarmac.no-ip.org'. you'll need your .ssh/config configured like https://pastebin.canonical.com/74130/ [13:05] ok, I'll take a look [13:05] bac: as user 'ubuntu' or as user 'tarmac' ? [13:08] jam: tarmac is easiest. either works and both have sudo. [13:12] jam: did you get access to the machine? [13:12] bac: otp now, will try later. [13:13] bac: I did. No lxc containers are running, no juju environment was bootstrapped. I bootstrapped the test env and it seemed to work fine. (and then I destroyed it) [13:14] is there some way I can re-run the merge request? [13:15] benji: yes. the log file is in ~/.config/tarmac/tarmac.log. grep for Merging and it'll show you the branch name [13:15] cd ~/repos/lpsetup/trunk [13:15] hey benji, wanna use tb? [13:17] bac: termbeamer is up and sending you my session [13:42] bac: juju appears to be hung waiting for the unit to come up [13:43] juju status times out [13:44] bac: ello when do you guys have your stand up? [13:46] czajkowski: 1600Z [13:46] why? [13:47] benji: that's ungood. [13:48] benji: we could bounce the instance but that doesn't solve the problem. [13:50] bac: yeah; I was hoping youhad seen this before; let me investigate some more and see if I can get closer to a root cause [13:52] bac: might be good to join a few in case we have over lap on stuff [13:52] given you're now on maintenance no ? [13:54] wgrant: Yup, but still waiting for a point release or two. [13:55] benji: ok. if you ever want to bounce it, just use 'poweroff' in the instance and then the startup script in ./bin on your local machine [13:57] bac: where is the startup script? [13:58] in the config branch in bin [13:58] benji: lp:lp-tarmac-configs [13:58] cool, thanks [14:16] deryck: ping, when you get a sec then I want to sanity check the +newproduct ProjectGroup vs +new and Product [14:17] deryck: http://paste.mitechie.com/show/782/ for the quick rundown on the bits fitting together [14:26] rick_h_, looking [14:28] rick_h_, so looking at that, it seems to me you only care about getting allowed types in the second step if the context is IProduct. [14:29] rick_h_, or else you could have the same method on IProject that only returns Public. [14:29] deryck: right, ok that sounds good === al-maisan is now known as almaisan-away === matsubara is now known as matsubara-lunch === deryck is now known as deryck[lunch] [16:28] bac: how long should the inttance take to start? it has been around 30 minutes of "[localhost] local: scp -r -o StrictHostKeyChecking=no * 10.55.60.129:" [16:30] * benji gets some lunch. === Ursinha` is now known as Ursinha === Ursinha is now known as Guest37586 === deryck[lunch] is now known as deryck === Ursinha-afk is now known as Ursinha [17:18] lifeless, ping. wondering if my 10.4 second db patch, according to dbupragde.log is okay for deployment. [17:22] benji: mp for you if you get a sec: https://code.launchpad.net/~rharding/launchpad/pp_register/+merge/122666 [17:23] deryck: I left the fake attribute in there with an XXX for the db side since I figured it was more ugly to try to dynamically add in the @property conditionally [17:24] rick_h_, it's all behind a feature flag, too, right? [17:24] deryck: right [17:24] ok [17:24] rick_h_: sure; it will be a while though [17:25] benji no hurry, thanks [17:25] deryck: and since I'm using an @property and you'll have a db attribute there shouldn't be much to resolve no matter who gets in first for conflict stuff. [17:26] rick_h_, right. sounds fine to me, but I'll defer to benji for the real review. [17:26] deryck: right, just fyi your way [17:26] gotchas [17:40] bac: I can't get the tarmac container to start; do you mind trying for me? [17:43] benji: can you ssh to it? [17:47] bac: nope [17:47] benji: you did try by ip addr? [17:48] bac: nope; let me find it (I don't have it on hand) [17:49] found it [17:50] bac: ssh tarmac@10.55.60.123 fails with "Permission denied (publickey)." [17:51] benji: yep, don't use tarmac@. === matsubara-lunch is now known as matsubara [17:51] he's probably not created yet. if you don't specify you log in as ubuntu [18:04] bac: "ssh 10.55.60.123" still gives me "Permission denied (publickey)." [18:07] benji: uhg [18:39] deryck: definitely not [18:40] deryck: we're looking for 4-5 seconds max IIRC the math correctly. [18:40] deryck: which patch was it [18:40] ? [18:59] lifeless: I believe deryck meant https://code.launchpad.net/~deryck/launchpad/product-db-changes-for-privacy/+merge/122941 [18:59] lifeless, yeah, it's 2209-33-1, the specification_sharing_policy column. [18:59] lifeless, I have no idea how to get a single column add any faster. :) [19:00] lifeless, sorry for the delay, but I've got pool people here to give and estimate on my pool servicing. I need to step away again, but won't be too long. [19:11] deryck: you're fine, a close inspection shows we have a time reporting bug. [19:12] 2012-09-10 00:40:09 INFO Applying /srv/staging.launchpad.net/staging/launchpad/database/schema/patch-2209-23-1.sql [19:12] 2012-09-10 00:40:20 INFO Applying /srv/staging.launchpad.net/staging/launchpad/database/schema/patch-2209-31-1.sql [19:12] 23-1 took 11 seconds [19:12] I need to see what that is. [19:12] it might be wgrants index, in which case it went on concurrently on prod and we can ignore it. [19:14] deryck: which is is, so your patch is fine, it isn't 11 seconds long. [19:15] deryck: could you please file a bug about this? It will confuse others.. [19:15] probably fallout from slony [19:29] benji: i was able to restart the c'stack instance with no problem. did you update your env per the instructions on the internal wiki? [19:29] bac: by sourcing ~/.canonistack/novarc? if so, yes [19:30] benji: in .canonistack do you have a benji_lcy01.key or just benji.key? [19:30] bac: benji_lcy01.key [19:30] benji: ok, then, i'm not sure what the problem is [19:30] maybe later we can use a termbeamer to see what is going on [19:31] bac: that'd be great; ping me when you have a moment [19:32] deryck: Can we chat? [19:41] lifeless, sure, I can file a bug. [19:41] abentley, sure. [19:42] deryck: Okay, g+ hangout. [19:43] ok, coming now.... [19:44] gary_poster: I've fixed (just pushed to trunk now) a bug in testrepository causing test times to be badly undercounted when the stream isn't timed. I think this won't affect paralleltest due to the subunit stream from zope being timed, but if it isn't timed, then I'm wrong, and you would want the fix. [19:45] lifeless, cool! it is timed for us because we pass -vvv (or something silly like that) to bin/test, but might be helpful for us even in other situations. [19:47] https://bugs.launchpad.net/testrepository/+bug/1048126 for reference [19:47] <_mup_> Bug #1048126: Test times of untimestamped streams are bogus < https://launchpad.net/bugs/1048126 > [20:04] flacoste: hi ? [20:04] hi lifeless [20:04] we on ? [20:04] we are [20:05] hah [20:06] thats something skype handles better [20:06] when both sides call [20:06] I'll start a fresh fresh one [20:06] lifeless: hang on, i'm with google on the phone (search broken) [20:06] ok [20:07] grab me whenever [20:15] lifeless: can you send me a new invite? [20:16] sure [20:18] flacoste: you should have it... [21:04] benji: still around? [21:04] bac: just :) === benji changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: - | Firefighting: - | Critical bugs: ∞ [21:04] benji: https://plus.google.com/hangouts/_/ef48f894900cf80079fb2996e64e6b0ae17df36d?authuser=1&hl=en-US [21:05] bac: I'm afraid I'm away from my hangout setup; mind if we do it in the morning? [21:05] benji: nope. have a good evening. [21:05] thanks; you too! === spm is now known as stevemci === stevemci is now known as spm