| StevenK | wgrant: Hmmm, how do I return a 400. | 00:50 |
|---|---|---|
| wgrant | StevenK: Maybe raise a BadRequest, but I'm not sure if you can at that point. | 00:58 |
| StevenK | I just tried with HTTPBadRequest and that OOPSes | 00:59 |
| StevenK | wgrant: BadRequest also OOPSes | 01:06 |
| StevenK | I might be using the wrong one, since I'm pulling it from lazr.restfulclinet | 01:07 |
| StevenK | *restfulclient | 01:07 |
| wgrant | StevenK: HTTPBadRequest is probably the right one | 01:08 |
| wgrant | But the request may not be sufficiently set up by then | 01:08 |
| wgrant | What does the traceback look like? | 01:08 |
| StevenK | wgrant: http://pastebin.ubuntu.com/1595337/ | 01:09 |
| wgrant | StevenK: Ah, so the publisher deals with it really early | 01:10 |
| wgrant | Might best to go the decode('utf-8', 'replace') route | 01:10 |
| StevenK | Right, like PATH_INFO ? | 01:10 |
| StevenK | wgrant: My test might be screwing that up | 01:11 |
| StevenK | But given it's three lines, maybe not | 01:12 |
| wgrant | Like PATH_INFO, yes | 01:12 |
| wgrant | If they don't speak UTF-8 then they're not worth understanding :) | 01:13 |
| StevenK | wgrant: http://pastebin.ubuntu.com/1595363/ | 01:19 |
| wgrant | StevenK: Heh, no | 01:19 |
| StevenK | wgrant: Would you prefer _decode() turns into return unicode(text, 'utf-8 | 01:20 |
| StevenK | Bleh | 01:20 |
| wgrant | StevenK: Well, think about what your diff will do | 01:20 |
| StevenK | return unicode(text, 'utf-8', 'replace') even | 01:20 |
| wgrant | It'll go through all the user-specified charsets | 01:20 |
| wgrant | Decoding each with replacements | 01:20 |
| wgrant | Then decode as UTF-8 with replacements | 01:21 |
| wgrant | What you might want to do is try to decode as the user-specified charsets | 01:21 |
| wgrant | Then, if they all fail, decode as UTF-8 replacements | 01:21 |
| wgrant | *or* just go ahead and decode as UTF-8 with replace to start with | 01:21 |
| wgrant | I'd prefer the former | 01:21 |
| StevenK | wgrant: http://pastebin.ubuntu.com/1595366/ | 01:23 |
| wgrant | StevenK: No | 01:23 |
| StevenK | Oh, missing a break | 01:24 |
| wgrant | It's also wrong :) | 01:25 |
| wgrant | That'll decode as utf-8 with replace if any decoding fails | 01:25 |
| wgrant | You want to do it if *all* decodings fail | 01:25 |
| wgrant | So perhaps you want to | 01:25 |
| wgrant | def _decode(self, text): | 01:25 |
| wgrant | text = super(LaunchpadBrowserRequest, self)._decode(text) | 01:25 |
| wgrant | if isinstance(text, str): | 01:26 |
| wgrant | text = test.decode('utf-8', 'replace') | 01:26 |
| wgrant | return text | 01:26 |
| wgrant | s/test/text/ | 01:26 |
| StevenK | Heh | 01:26 |
| StevenK | wgrant: That sounds good. Even with an explanatory comment. | 01:28 |
| StevenK | BrowserRequest does not love adapting LBR | 01:29 |
| StevenK | So I'll call it directly | 01:29 |
| wgrant | ? | 01:29 |
| wgrant | Oh, is it not new-style? | 01:29 |
| StevenK | envadapter = IUserPreferredCharsets(self) | 01:30 |
| StevenK | TypeError: ('Could not adapt', <lp.services.webapp.servers.LaunchpadBrowserRequest instance URL=http:/>, <InterfaceClass zope.i18n.interfaces.IUserPreferredCharsets>) | 01:30 |
| wgrant | Ah | 01:30 |
| wgrant | That's odd... | 01:30 |
| StevenK | We don't implment IBrowserRequest | 01:30 |
| StevenK | That might be it | 01:30 |
| === slank is now known as slank_away | ||
| wgrant | Except that we inherit that method and it works fine | 01:31 |
| StevenK | When I copied and pasted it, it was failing with the IUserPreferredCharsets line, but I just removed it rather than looking for the import | 01:32 |
| wgrant | Sure, but _decode is called today and seems to work fine, right? | 01:32 |
| StevenK | LBR doesn't have a _decode today | 01:34 |
| StevenK | It will fall to the subclasses | 01:34 |
| wgrant | Exactly | 01:34 |
| wgrant | Which have the same self | 01:34 |
| wgrant | But work fine | 01:34 |
| StevenK | Ah, hm | 01:34 |
| StevenK | implementing IBrowserRequest helps not | 01:34 |
| StevenK | Neither does calling text = BrowserRequest._decode(self, text) | 01:36 |
| wgrant | StevenK: So you're getting that crash in a browser? | 01:38 |
| StevenK | That's from my test | 01:38 |
| StevenK | Let me try the browser | 01:38 |
| wgrant | Oh | 01:39 |
| wgrant | You might not even be in the right layer | 01:39 |
| StevenK | TestBasicLaunchpadRequest has no layer | 01:40 |
| wgrant | That would be a problem for a test that desires to make use of the Component Architecture. | 01:40 |
| StevenK | TestLaunchpadBrowserRequest doesn't either | 01:40 |
| StevenK | wgrant: It works in the browser, so yeah, layers | 01:41 |
| StevenK | None of the testcases in services/webapp/tests/test_servers use layers at all | 01:42 |
| wgrant | Right, none of them need to at present | 01:43 |
| StevenK | LFLayer, or is that too high? | 01:43 |
| wgrant | FunctionalLayer should be fine | 01:44 |
| wgrant | Don't even need DatabaseFunctionalLayer | 01:44 |
| StevenK | wgrant: http://pastebin.ubuntu.com/1595398/ | 01:46 |
| wgrant | StevenK: Might want to extend the comment to state that we always want unicode, dammit. | 01:47 |
| StevenK | wgrant: http://pastebin.ubuntu.com/1595412/ | 01:49 |
| wgrant | StevenK: Seems reasonable. | 01:51 |
| wgrant | Checked that it fixes the crash? | 01:51 |
| StevenK | It renders the page with the replaced string in the widget | 01:51 |
| StevenK | So 'good enough' | 01:51 |
| wgrant | Yep | 01:52 |
| wgrant | StevenK: I'll have ~3600 lines of diff for you soon :) | 01:58 |
| wgrant | Over 6 branches, though | 01:58 |
| StevenK | Whimper | 01:58 |
| wgrant | :) | 02:05 |
| wgrant | StevenK: So, do you understand what I'm doing? | 02:17 |
| wgrant | https://code.launchpad.net/~wgrant/launchpad/flatten-bfj-1-populate/+merge/145541 https://code.launchpad.net/~wgrant/launchpad/flatten-bfj-2-garbo/+merge/145542 https://code.launchpad.net/~wgrant/launchpad/flatten-bfj-3-query/+merge/145543 https://code.launchpad.net/~wgrant/launchpad/flatten-bfj-3.5-more-query/+merge/145819 https://code.launchpad.net/~wgrant/launchpad/flatten-bfj-4-app-eliminate/+merge/145820 | 02:36 |
| wgrant | StevenK: They're ready for you to review at your leisure | 02:37 |
| StevenK | wgrant: You never explained your dastardly plan | 02:55 |
| wgrant | StevenK: I've outlined it in the MP descs | 03:04 |
| wgrant | But can go into more detail if you desire | 03:04 |
| StevenK | wgrant: Yeah | 03:05 |
| wgrant | To which? :) | 03:05 |
| StevenK | The first comment | 03:06 |
| wgrant | Ah, good :) | 03:06 |
| wgrant | It's conceptually simple, there's just a lot of code to change | 03:06 |
| StevenK | wgrant: I've approved -1-populate with one niggle, but it took me a few minutes to figure out the the _new_* gubbins | 03:06 |
| StevenK | Hopefully they lose their _new_ness | 03:06 |
| wgrant | Yes | 03:06 |
| wgrant | They're just there so they don't override the properties which delegate to PB/BFJ | 03:07 |
| wgrant | So all the other callsites don't have to care | 03:07 |
| wgrant | (since the _new_ cols obviously won't be populated when the appservers first start up with that code) | 03:07 |
| wgrant | They'll be renamed in -5, I suspect | 03:07 |
| StevenK | Yeah | 03:07 |
| StevenK | -5-scorched-earth ? :-) | 03:08 |
| * StevenK hits line of 525 of -3.5 and vomits | 03:29 | |
| wgrant | Mmm? | 03:30 |
| StevenK | 520+ BinaryPackageBuild, Archive).find( | 03:30 |
| StevenK | 521+ BinaryPackageBuild, | 03:30 |
| StevenK | 522SQL(query)) | 03:30 |
| StevenK | 523resultset.order_by( | 03:30 |
| StevenK | It's sort of disgusting | 03:31 |
| StevenK | And needs a rewrite | 03:31 |
| wgrant | Ah yeah, a bit, but not in this branch :) | 03:31 |
| wgrant | It needs Stormification | 03:31 |
| wgrant | Which should come as part of the search rewrite in -6 | 03:31 |
| StevenK | Heh | 03:31 |
| StevenK | Which branch does DROP TABLE PackageBuild, then? | 03:32 |
| wgrant | -7 or so, because it's not quite so urgent :) | 03:32 |
| StevenK | Maybe -8 can make retry-depwait a garbo-frequently job | 03:33 |
| * StevenK hides | 03:33 | |
| wgrant | No | 03:33 |
| wgrant | It takes too long | 03:33 |
| wgrant | It's already a tunable loop, and could maybe be in -hourly | 03:34 |
| StevenK | Even with this change it will be too slow? | 03:34 |
| wgrant | Yes | 03:34 |
| wgrant | This won't affect it in any significant fashion | 03:34 |
| StevenK | wgrant: Up to -4 | 03:36 |
| wgrant | StevenK: Right, I originally considered having a common garbo base, but the column lists and joins for each are somewhat different, and the code will be on prod for just hours | 03:39 |
| StevenK | Right, exactly why I ended up with 'eh' | 03:39 |
| StevenK | wgrant: I'm done, all approved. -3.5 has the most niggles | 03:40 |
| wgrant | Thanks | 03:40 |
| wgrant | StevenK: Can you reapprove https://code.launchpad.net/~wgrant/launchpad/flatten-bfj-3-query/+merge/146035 (no changes other than a merge from a garbo-dropping branch, which required a resubmit) | 03:54 |
| wgrant | Your first comment will be resolved in a later branch that rewrites the search stuff, and your second is invalid (see the next couple of lines) | 03:55 |
| StevenK | 1013+ find_spec = find_spec + (order_by_table,) | 03:55 |
| StevenK | OH | 03:55 |
| StevenK | That's hideous | 03:56 |
| wgrant | Quite! | 03:56 |
| StevenK | wgrant: Re-approved | 03:56 |
| wgrant | Thanks | 03:56 |
| StevenK | wgrant: Blink. My test failed on buildbot, but passes locally. | 04:04 |
| StevenK | 2.7 versus 2.6, at a guess? | 04:04 |
| wgrant | StevenK: Yeah | 04:05 |
| wgrant | 2.6 eats the whole character | 04:06 |
| wgrant | It assumes that the size indicated by the first byte is correct. | 04:06 |
| StevenK | Hm, not sure how to deal with this test for both 2.6 and 2.7, then. :-( | 04:07 |
| wgrant | http://bugs.python.org/issue8271 | 04:07 |
| StevenK | wgrant: Ouch | 04:12 |
| StevenK | wgrant: assertTrue(...['field.title'].startswith('...')) ? | 04:12 |
| wgrant | StevenK: I'd just check if it's one of the two known values | 04:12 |
| wgrant | With an XXX | 04:13 |
| StevenK | Right | 04:13 |
| StevenK | wgrant: http://pastebin.ubuntu.com/1595628/ | 04:19 |
| wgrant | StevenK: Looks good | 04:21 |
| StevenK | wgrant: -0 is waiting for stub, and I guess -1 can't land until after a FDT? | 04:26 |
| wgrant | StevenK: Right. | 04:26 |
| wgrant | Most of the fun will happen after an FDT at 1pm on Monday, I suspect | 04:26 |
| wgrant | They're all in ec2 now | 04:27 |
| StevenK | How many indicies do you add? | 04:27 |
| wgrant | (the runs are condemned due to your failure, though) | 04:27 |
| wgrant | Um, not sure at this point | 04:27 |
| wgrant | That's for working on between 2.5 and 3 on Monday | 04:27 |
| wgrant | Since I can approve them myself and get ops to apply them now | 04:27 |
| StevenK | Last time I looked, your DB patch said TODO: add indicies | 04:27 |
| wgrant | I removed that while you were reviewing the rest, I think | 04:27 |
| StevenK | Ah | 04:28 |
| wgrant | There's no point creating them at that stage | 04:28 |
| wgrant | Quicker to create them at the end when all the data's there | 04:28 |
| StevenK | Indeed | 04:28 |
| StevenK | And it's not my failure, it's all 2.6's fault. | 04:28 |
| wgrant | True | 04:28 |
| StevenK | wgrant: Shall we deploy in about 70 minutes? | 04:29 |
| StevenK | Might be closer to 80, but eh | 04:29 |
| wgrant | So, remaining changes: work out indices, clean up all the _new crap, drop PackageBuild itself, drop the old DB columns/table, apply various NOT NULL constraints, work out indices for the new D/DS/SPN/i_d_a queries, integrate the new D/DS/SPN/i_d_a queries | 04:30 |
| wgrant | And that should be that | 04:30 |
| wgrant | And yes, we should deploy tonight | 04:30 |
| wgrant | Will close 3 criticals? | 04:30 |
| StevenK | Yah | 04:31 |
| StevenK | I need to find another | 04:31 |
| StevenK | wgrant: How can I pull apart this checkwatches core ? | 04:40 |
| wgrant | You may not have much luck pygdbing it post-portem | 04:41 |
| wgrant | post-mortem | 04:42 |
| wgrant | Ideally we'd do it in-place | 04:43 |
| StevenK | On loganberry? | 04:45 |
| wgrant | Yes | 04:46 |
| wgrant | stub: Thanks. Those indices were indeed planned for a quick followup, but the DEFAULT thing slip my mind this time. | 06:45 |
| stub | yup, I assumed the indexes would be landing soon. Just making sure those particular ones don't get lost in case you were mainly thinking about performance tuning. | 07:16 |
| wgrant | stub: Though it doesn't matter much for librarian-gc anyway | 07:17 |
| wgrant | As it basically seqscans everything | 07:17 |
| wgrant | But still useful | 07:17 |
| wgrant | It's person merges that really matter, as they are selective | 07:18 |
| stub | mmm... | 07:18 |
| stub | I haven't looked for ages. I was thinking there was some selectivity in there. | 07:18 |
| wgrant | Well, consider what it has to do :) | 07:19 |
| wgrant | It needs to find LFAs that are unreferenced | 07:19 |
| wgrant | Which means finding a list of everything that is referenced | 07:19 |
| wgrant | The index only helps a lot if the column is relatively sparse | 07:20 |
| stub | That is one part of the algorithm. There used to be parts that benefited from the indexes. But TBO, I don't really know if that is still the case after a few years of tweaking. | 07:32 |
| stub | If nothing else, deleting a single row from LFA requires looking up if that row is referenced anywhere - index lookups or table scans if the indexes don't exist. | 07:33 |
| stub | Which is hundreds or thousands each gc run | 07:34 |
| wgrant | Oh yeah, that bit is a good point. | 07:39 |
| wgrant | Duh | 07:39 |
| === Masklinn_ is now known as Masklinn | ||
| === yofel_ is now known as yofel | ||
| === Ursinha is now known as Ursinha-afk | ||
| === matsubara-afk is now known as matsubara | ||
| === Ursinha-afk is now known as Ursinha | ||
| === Ursinha is now known as Ursinha-afk | ||
| === Ursinha-afk is now known as Ursinha | ||
| === Ursinha is now known as Ursinha-afk | ||
| === salgado is now known as salgado-brb | ||
| === salgado-brb is now known as salgado | ||
| === matsubara is now known as matsubara-lunch | ||
| === slank_away is now known as slank | ||
| === Ursinha-afk is now known as Ursinha | ||
| === Ursinha is now known as Ursinha-afk | ||
| === Ursinha-afk is now known as Ursinha | ||
| === matsubara-lunch is now known as matsubara | ||
| === deryck is now known as deryck[lunch] | ||
| === Ursinha is now known as Ursinha-afk | ||
| === Ursinha-afk is now known as Ursinha | ||
| === mpt_ is now known as mpt | ||
| === Ursinha is now known as Ursinha-afk | ||
| === deryck[lunch] is now known as deryck | ||
| === matsubara is now known as matsubara-afk | ||
| === Ursinha-afk is now known as Ursinha | ||
| === Ursinha is now known as Ursinha-afk | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!