=== beuno_ is now known as beuno [00:15] wallyworld_: I've had breakfast, but I think I need at least one cup of tea before our chat. [00:16] StevenK: np [00:16] Also got distracted by finishing my double-word-death branch [00:16] * StevenK tosses it at ec2 [00:18] wgrant: ? [00:19] lifeless: You said "heh, doit." but didn't actually vote. [00:20] twill be clear to onlookers :> [00:20] And the subsequent 6 lines were about that tag subscription bug that you commented on. [00:20] ah yes [00:20] which is beautiful win [00:25] wallyworld: Sorry, now I'm waiting for the jerk with the leaf blower to move [00:25] StevenK: what? can't hear you over the noise [00:26] Or I get annoyed enough to go downstairs and ... re-position the nozzle [00:26] lifeless: Can we migrate to SQLAlchemy now please? [00:26] Add it sourcedeps. I dare you. [00:27] wgrant: raise it on the list [00:27] Hah [00:27] lifeless: You have no objections? [00:27] * StevenK deals lifeless a penalty card for 'Lying' [00:28] StevenK: oh? [00:28] StevenK: deal yourself one [00:28] StevenK: then ask Francis what I've said in the past. [00:28] lifeless: You have strong opinions on everything we use, so I find it hard to believe you don't have one about this. [00:28] I never claimed no opinion [00:28] wgrant: is my new hero for that comment [00:39] if( vs if ( [00:40] pep8 says if ( [00:42] wallyworld: http://pastebin.ubuntu.com/852074/ [01:01] * StevenK stabs Firefox === 13WAAIDLL is now known as wallyworld === wallyworld is now known as Guest76072 [02:04] wgrant: it looks like bulk.create with a 0-length values will perform an execute and get a ProgrammingError. What do you think about changing it to handle 0-length values by returning? [02:05] wgrant: Also, is it efficient to load when the values aren't needed? [02:06] abentley: Much like the rest of Storm's SQL compilation stuff it is a bit fragile, yeah. [02:06] And loading should indeed probably be made optional. [02:06] So, yeah, 0-length values should probably just be a no-op, and loading should be disablable. [02:07] wgrant: Cool. [02:07] wgrant: The 0-length values is the only reason I kept insert_many in my latest branch. [02:07] abentley: I'm intrigued as to how your experiment was so slow. [02:07] Perhaps sqlvalues is awful. [02:08] abentley: How fast is the original code, btw? [02:08] Without any bulk optimisations. [02:08] wgrant: I'm curious too, but I've got lots of things piling up at the moment. [02:08] s/fast/slow./ [02:08] Yeah [02:09] wgrant: I took over from gmb, so I didn't try it. I know it exceeded the branch scanner job timeout. [02:09] Heh [02:09] which bit of the scanner is timing out? [02:09] the wrapper [02:09] Both BranchRevision and Revision insertion have been problematic. [02:09] ok [02:10] And there was a libreoffice non-DB timeout yesterday. [02:10] I didn't investigate. [02:10] mwhudson: The db insertions of Revisions and BranchRevisions. [02:10] i worked on BranchRevision pretty hard at one point [02:10] as you have probably noticed [02:10] Yeah [02:10] IIRC it uses manual string SQL now? [02:10] Like the rest of LP's existing bulk inserts. [02:10] Which I'm going to port to the new shiny tomorrow, probably. [02:10] yeah [02:10] that'll be good [02:10] wgrant: Are you fixing the same bug as me? [02:10] (even if it doesn't end up being faster) [02:10] abentley: No. [02:10] abentley: I'm working on disclosure stuff. [02:11] abentley: jelmer is working on the bzr side of scanner timeouts, though. [02:11] wgrant: I've ported it over to insert_many already. [02:11] abentley: I informed him of your potential conflicts. [02:11] abentley: Yeah, but there are other places that do manual bulk inserts. [02:11] BPPH creation and a few others. [02:11] wgrant: Oh, I misread you. [02:11] I'm not going to touch your stuff :) [02:12] wgrant: Well, if you insist on doing it better than me, I'll grumble, but not too much :-) [02:12] Heh [02:13] abentley: You've wrapped create() rather than fixed it directly? [02:13] The initial version will land in about 90 minutes, so I'll land a quick followup with the changes you suggested unless you already have one. [02:13] wgrant: No, I hadn't seen create. I was using "Store.execute(Insert( ..." [02:13] Ah [02:14] I'll make those two changes now, then. [02:14] Thanks. [02:15] Hopefully this will make people avoid bulk creation a bit less. [02:15] 'cause there are a lot of places that could use it. [02:16] abentley: I apologise for colliding with you. I didn't realise you were working on this until I was proposing my Storm branch and saw yours. [02:17] wgrant: No worries. Strange coincidence that we both tried to fix this longstanding gap in the same month. [02:18] Indeed. [02:21] * wgrant benchmarks the old one too, just for curiosity's sake. [02:24] wgrant, jelmer: In last week's profiles of lp:s~irar/gcc-linaro/slp-for-any-nops-4.6/, only 13% of branch scanner time was spent in getBazaarRevisions, so I wasn't too concerned about it. [02:25] wgrant, jelmer: Though wgrant's bulk-insert will make the db side faster. [02:25] wgrant@lucid-test-lp:~/launchpad/lp-branches/devel$ ls -lh /var/tmp/lperr [02:25] total 191M [02:25] drwxr-xr-x 2 wgrant wgrant 189M 2012-02-22 13:24 2011-12-05 [02:25] That can't be good... [02:27] A 189M dentry? Niiiiiiiiice [02:27] Yeah [02:27] I think that's the day I reproduced the poppy rabbit explosion. [02:27] Which could mean there are millions of files in there. [02:27] Likely [02:28] ls -1 /var/tmp/lperr/2011-12-05 | wc -l ? :-) [02:28] I unfortunately already deleted part of it, but I'm already doing that. [02:28] Been going for 3 minutes... [02:28] Haha [02:28] It's probably statting all the children :*( [02:28] It will, yes [02:29] Might write something to just readdir instead. [02:29] Oh no [02:29] It's still calling getdents [02:29] 512 at a time [02:29] Bwahaha [02:30] ls is only using 3% of my RAM so far, though. [02:40] Ew [02:40] At least 4 statements per Revision insert in devel. [02:41] And often 7 [02:41] And oh look it timed out. [02:42] Less than half way through. [02:46] * wgrant starts it without a timeout and goes away for 15 minutes. [02:51] Huh [02:51] It doesn't really seem to be inserting them in any kind of order. [03:06] abentley: Just tested on lp:launchpad. 9:00 without bulk inserts, 1:45 with mine. [03:07] Probably doesn't improve the normal case much, but the initial scan of a large new history is 5x faster even with no DB latency. [03:13] Yeah, doesn't help the 0:47 new branch with existing Revisions case at all. [03:15] mwhudson: Did you ever look at sharing history? [03:17] eg. bisect to find the latest shared history and then delegate to that. [03:29] wgrant: it was examined, based on the work jam had done in history-db [03:30] lol, pybars has had 65 downloads [03:30] interesting [03:30] s/interesting/google/ [03:30] you think? [03:31] * wgrant finds out. [03:34] wgrant: for branch revision? [03:34] mwhudson: Yeah [03:34] It would presumably reduce the table size by something like 99%. [03:35] lifeless: Oh, bah, hosted on PyPI rather than LP? [03:35] No referers for me :( [03:36] pypi -> centre of the python universe [03:36] Yeah, but often our downloads are hosted on LP. [03:38] wgrant, lifeless: Can I Disapprove https://code.launchpad.net/~linaro-infrastructure/launchpad/workitems-migration-script/+merge/93883 ? [03:38] garbo job or API script, damn it [03:39] As I said a couple of days agoo, I think it should be an API script. But it needs discussion. [03:39] I recommended garbo to salgado a week or two back [03:40] All of three of us are in agreement about "Oh god, not that>" [03:40] s/>/./ [03:40] I think its reasonable to say that you want to discuss it [03:40] and yes, I agree, API or garbo makes much more sense [03:42] lifeless: "I'm going to disapprove this. This requires discussion, and certainly doesn't require a one-shot script -- the usual modus operandi for migrations like this is either a garbo job or an API script." [03:46] lifeless: Sound fine? [03:46] wgrant: i'm not sure how that would help things like mapping a revision to a branch [03:46] StevenK: personally, I would needs-info rather than disapprove, but if you're reviewing, do what you prefer [03:46] you can do cleverer things that BR of course, but you end up doing something like historydb i think [03:46] StevenK: I'll note that its a tunableloop already [03:46] there is a wiki page somewhere about being cleverer than BR, come to think of it [03:47] StevenK: which means it can trivially sit in the garbo [03:47] wallyworld__: Om nom nom XSS [03:47] hmm? [03:47] Your new sharing picker thing is nice and XSSable. [03:47] lifeless: Meh, needs fixing [03:47] And not feature-flagged :( [03:47] StevenK: sure, or that [03:48] wgrant: it's on a view that is not used [03:48] wgrant: https://dev.launchpad.net/Code/BranchRevisions [03:48] wallyworld__: That's only barely mitigation. [03:48] wallyworld__: if someone can craft a url to it, they can exploit it [03:48] (if there is an xss hole...) [03:49] i'm not sure if the view is featured flags or not [03:49] It's not. [03:49] so we need to ff the view [03:50] lifeless, wgrant: You both chimed in on https://code.launchpad.net/~sinzui/launchpad/team-titles/+merge/93675 ? Do you have any remaining objections or were you just commenting? [03:50] StevenK: sinzui took my suggestion into account [03:50] I'm OK as long as the bug title changes are reverted. [03:50] StevenK: the title is now preserved - see his QA instructions [03:51] wgrant: they aren't reverted, but titles are shown in the page title [03:53] lifeless: i'm in iharness and using Launchpad.login_with(). but there's no get() or named_get() method on the lp instance that is returned. what's the best way to get an instance of WebServiceCaller with those methods? [03:53] Launchpad.login_with() is a full launchpadlib [03:53] Heh [03:54] This is actually a two-layer XSS [03:54] Cool [03:54] wgrant: Are you okay with the MP given what lifeless said? [03:54] wgrant: there's no get or named_get methods though it seems [03:54] wallyworld__: No, launchpadlib uses attribute access instead. [03:55] so how to i get something i can invoke named_get or get on? [03:55] Do you want to do that, or do you just want to experiment with the API? [03:56] i have a service i think have exposed and i want to try and call it [03:56] Write a webservice test, then? [03:56] Might as well just use launchpadlib. It's a nicer interface than WebServiceCaller [03:56] And I don't know if one can use WebServiceCaller from outside AppServerLayer [03:57] lp = Launchpad.login_with(fewfwefwehfuwef) [03:57] obj = lp.load('/path/to/your/resource') [03:57] obj.someattribute [03:57] obj.someMethod(some=arg) [03:57] .load is what i was missing [03:57] it's not a domain object though [03:57] Have you regenerated your WADL? [03:57] yes [03:57] i'm treating it as a service [03:58] You should be able to access it by lp. [03:58] But note it's the top-level collection name, not its URL. [03:59] so i assume export_as_webservice_entry() is the correct way to register it [03:59] wallyworld__: Anyway, these XSSes are now blocking deployment. [03:59] No. [03:59] Collections aren't entries. [03:59] it's not a collection [03:59] it's a service [03:59] Hm, so it's a singleton entry? [03:59] I guess that's OK, then. [04:00] These things generally get exported as collections, but I guess your way makes more sense. [04:00] the ws api is geared to export entries and collections sadly [04:00] Just means there's no way to access it without lp.load [04:00] ideally i'd like to say something like lp.get('/myservicename?ws.op=something') [04:01] or lp.post(/myservicename?ws.op=something&data=foo) [04:01] it that possible? [04:04] wgrant: so where's the xss hole? in the picker config passed to the client from the view? [04:04] wallyworld__: LaunchpadWebServiceCaller is the interface you get in tests. [04:04] It looks like it speaks TCP [04:04] So you can probably use it. [04:05] LaunchpadWebServiceCaller(protocol='https') should work [04:05] I hope [04:06] wallyworld__: so why do you want that rather than lp.thing.something() and lp.thing.something(foo) ? [04:06] lifeless: whatever works. i tried that and couldn;t get it to work [04:06] that would be my preferred way [04:06] Oh [04:06] Then make that work :) [04:06] What's the error? [04:07] wallyworld__: so, suggestion, and I'm not being snarky - don't talk about what you don't want. Talk about what you do want! [04:07] sure. [04:07] the error is that lp has no attribute thing [04:07] What's the expression? [04:07] * StevenK distracts wgrant with something shiny [04:08] StevenK: Oh? [04:08] wgrant: Are you okay with the MP given what lifeless said? [04:08] so i'm not registering something properly. actually it's using the system launchpadlib [04:08] * wgrant reads the diff [04:08] wallyworld__: You sure you're connected to dev and not prod? [04:09] yep lp = Launchpad.login_with('testing', service_root='https://api.launchpad.dev', version='devel') [04:09] And what is failing, and what is the exact text of the error? [04:09] object has no attribute 'accesspolicies' [04:09] Right, you didn't register it as a top-level collection, so it won't appear there. [04:09] clearly i haven't set something up correctly [04:10] You have to use lp.load to get at it. [04:10] so how do i avoid the lp.load() step? [04:10] You can't. [04:10] Unless you say define it as an empty collection. [04:10] well [04:11] ideally lp.accesspolicies would map to my AccessPolicyService [04:11] what are the methods that you will have on this thing [04:12] at the moment, i have foo() [04:12] just to test it [04:12] ok, but really [04:12] help me understand [04:12] StevenK: Bug titles are missing a colon [04:12] It's now "Bug #1 blah blah blah" [04:12] ok, getAccessPoliciesForProducts(product_collection, user) [04:12] <_mup_> Bug #1: Microsoft has a majority market share Which doesn't make sense. [04:13] lifeless: so i want to go lp.accesspolicies.getAccessPoliciesForProducts(product_collection, user) [04:13] or something like that [04:13] wgrant: Right, noted in my +1, thanks [04:14] lifeless: and in the browser, there'd be an XHR call to the same api to get the data for the view [04:14] wallyworld__: you want to pass *in* a vector of products? [04:14] a set based interface [04:15] but there'd be just the one normally [04:15] so, I'm hugely in favour of set based calls, as you know [04:15] but all our existing api stuff would just make that a method on IProduct and IProject and IDistribution [04:15] noooooooo [04:15] (or IPillar probably) [04:15] putting service methods in domain objects is just so wrong [04:16] and i'd rather avoid that in this new tranch of work if possible [04:17] so i want to try a service based approach [04:17] That sounds good, but I htink then you need to commit to fixing the environment so that your new work is no worse than the current (which swimming upstream would be) [04:18] specifically, you'll need to teach launchpadlib about named objects or some such [04:18] one hack you could use is to create a collection of services - IService or something [04:18] right, that's what i didn;t know - i didn't realise it couldn't do that [04:18] Or just define this as a collection with [] as the default content. [04:18] collections have methods too [04:19] you'd need to fix the launchpadlib bug where the interface the wadl specifies the type of collection members even when they return a more specific type (and self-annotate as such) [04:19] I'm not actually sure if /branches is usefully iterable, for example. [04:19] or as wgrant says, an empty collection will get you a named service fairly cheaply [04:19] and i'd still need .load() right? [04:20] No [04:20] Top-level collections show up as lp.foo [04:20] wallyworld__: note though that the web API /only/ exports domain objects as far as its concerned, there isn't a separation there - and in most restful things I've seen there isn't a service layer as such [04:20] See eg. IBranchSet. [04:20] wallyworld__: so I'm not-at-all-sure your approach makes sense for the *external* API. [04:20] wallyworld__: I'd kind of like to talk it through with you and curtis [04:20] wallyworld__: I'd like to understand the vision [04:21] wallyworld__: where you want to take it [04:21] lifeless: i haven't talked to much to curtis about it [04:21] lifeless: want to jhoin our standup tomorrow? [04:21] This is very un-lazr.restful-ish. [04:21] But I think that's probably a good thing. [04:21] i'm experimenting a bit first to see what can be done [04:22] wallyworld__: I probably have conflicting calls; I have 3 in a row; what time is it ? [04:22] 8:00am AEST [04:22] So 11:00am [04:22] Damn Queenslanders. [04:22] Oohoohoo [04:22] 11am? surely you jest, he's not 3 hours out from you is he ? [04:22] Just realised what time the TL call is going to be for bigjools. [04:23] lifeless: He's 3 hours from you. [04:23] so, 11am I can do [04:23] One hour from us. [04:23] wgrant: Do share? [04:23] 6am [04:23] at the moment [04:23] lifeless: 6am for you, right? [04:23] no [04:23] Which makes it 3am for bigjools :) [04:23] Oh [04:23] 9am at the moment [04:23] hugely civilised [04:23] When did that change? [04:24] Disappointing. [04:24] when mrevell said roughly 'uhm, I need to put my kids to bed at the current time, can we change it' [04:24] lifeless: so i want to try and adopt a service oriented approach, using popo for the business model objects, separate from the storm db layer objects, and views getting flattened data from api calls etc [04:25] and still support "sensible" api access from launchpadlib [04:26] so this in some ways fits with where I want to take LP itself: remember I want to gut the server render layer to have no DB access at all [04:26] it's how i've always built these types of systems previously [04:27] however I think it would be good to separate the discussion into what you want the API to do and look and feel, and what you want to do in the appserver; because all the appserver work will be irrelevant as we move to more SOA backends [04:27] thats not a reason not to do appserver work - because some things pay for themselves very quickly, and improvements now are improvements now [04:27] but I still want to get a handle on the specifics you're intending [04:27] sure, np. it's still handwavy [04:28] and how we'll measure the success of the experiment; and how we'll roll all the way forward, or roll back (depending on the assessed success) [04:28] Anyway, we need to revert this picker thing. [04:28] wgrant: what's the xss issue? [04:28] wallyworld__: Put some stuff in a product title [04:28] s/title/displayname/ [04:28] And visit +sharing [04:29] so the json from the view doesn't get escaped properly? [04:29] Correct. [04:29] i thought it did? [04:29] You may recall that I strongly discourage ever encoding JSON manually. [04:29] And always using JSONRequestCache wherever possible. [04:30] wgrant: rather than revert, it's a simple one line fix to get the product title out of the picker header [04:30] That's not a fix. [04:30] why? [04:30] It's getting the one bit of bad data that I've found so far out of the unescaped section of the page. [04:31] Rather than removing the injection of user-provided data into unescaped section of the page. [04:31] we do that same pattern in a few other places in lp too [04:31] Yes, and they're all bugs. [04:31] eg the inline picker widget [04:32] Just because something is embarrasingly terrible already doesn't mean we should perpetuate it. [04:32] sure. that's the trouble when it's in the code - the pattern can be reused [04:32] Hmm, I'm sure I've posed that point before and you've debated it :) [04:33] so given the product title is the only user editable exploit, we could simply remove that for now [04:33] wallyworld__: it is the problem, and thats why we need to finish our transitions and migrations more [04:34] lifeless: well, i'm sure people thought the lazr picker work was finished when they did it [04:34] wallyworld__: software is never finished ;) [04:34] that's an answer to a slightly different question [04:34] wallyworld__: we can only hope to put some of it in the ground soon :) [04:35] wallyworld__: ok, so what was the question ? [04:36] i was merely saying that the work in question was probably considered to be "finished", without the expectation there was still stuff to do [04:36] It was finished. [04:36] It was just wrong. [04:36] fg [04:36] No jobs running [04:36] wgrant: or the jsonrequestcache encoder could be used in this case [04:37] rather than stuffing new stuff in the cache [04:38] so the json would be properly escaped [04:38] That's what we've done in the past. [04:38] But nobody remembers. [04:38] This is why manual encoding is no longer permitted. [04:39] i'll do that then for now [04:39] (this is all because Microsoft are fucking morons, btw) [04:40] so manual encoding works if done properly using the right encoder [04:40] Yes. [04:40] But I wasn't going to say that, because it's always a bad idea. [04:41] and in this case, i forgot to use the right encoder [04:41] That's why it's always a bad idea. [04:41] wonder if we patch something to force the correct encoder to be used by default [04:41] There's also a second layer of XSS after this. [04:41] So please just revert the whole thing :) [04:42] wgat's the other xss? [04:42] (an XSS I pointed out a few months ago during code review, but I was assured it would never be a problem) [04:42] which is? [04:42] The picker title is treated as HTML. [04:42] So even once you encode the JSON properly, the product title is then injected unescaped into the picker. [04:43] => revert it all until someone looks through it thoroughly. [04:43] i'd have to look to check, but the picker title is in the json data so should be escaped [04:43] Bonus points for deleting the rest of the pickers, but that seems unlikely. [04:44] That's not how escaping works :) [04:44] hmm? we use the correct encoder and the json data is all properly escaped [04:44] Right, when using the correct encoder the JSON is escaped fine. [04:45] But the JSON isn't really relevant for the second one. [04:45] so that's what i was saying - i'd have to check but i thought the picker title was in the json data [04:45] so would be escaped [04:45] It is. [04:45] That doesn't mean it's escaped properly in the picker. [04:46] but if it comes from json data that is escaped.... [04:46] * wgrant reverts it. [04:46] hang on a minute [04:47] the issue can be easily corrected using the proper encoder [04:47] The first issue. [04:47] and thye second issue is moot [04:47] Oh? [04:47] It happens, so it's not moot. [04:48] it is since the title is escaped in thje json [04:48] The JSON is escaped. [04:48] The title is not escaped in the JSON. [04:48]