[00:50] <StevenK> wgrant: Hmmm, how do I return a 400.
[00:58] <wgrant> StevenK: Maybe raise a BadRequest, but I'm not sure if you can at that point.
[00:59] <StevenK> I just tried with HTTPBadRequest and that OOPSes
[01:06] <StevenK> wgrant: BadRequest also OOPSes
[01:07] <StevenK> I might be using the wrong one, since I'm pulling it from lazr.restfulclinet
[01:07] <StevenK> *restfulclient
[01:08] <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:09] <StevenK> wgrant: http://pastebin.ubuntu.com/1595337/
[01:10] <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:11] <StevenK> wgrant: My test might be screwing that up
[01:12] <StevenK> But given it's three lines, maybe not
[01:12] <wgrant> Like PATH_INFO, yes
[01:13] <wgrant> If they don't speak UTF-8 then they're not worth understanding :)
[01:19] <StevenK> wgrant: http://pastebin.ubuntu.com/1595363/
[01:19] <wgrant> StevenK: Heh, no
[01:20] <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:21] <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:23] <StevenK> wgrant: http://pastebin.ubuntu.com/1595366/
[01:23] <wgrant> StevenK: No
[01:24] <StevenK> Oh, missing a break
[01:25] <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:26] <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:28] <StevenK> wgrant: That sounds good. Even with an explanatory comment.
[01:29] <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:30] <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:31] <wgrant> Except that we inherit that method and it works fine
[01:32] <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:34] <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:36] <StevenK> Neither does calling text = BrowserRequest._decode(self, text)
[01:38] <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:39] <wgrant> Oh
[01:39] <wgrant> You might not even be in the right layer
[01:40] <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:41] <StevenK> wgrant: It works in the browser, so yeah, layers
[01:42] <StevenK> None of the testcases in services/webapp/tests/test_servers use layers at all
[01:43] <wgrant> Right, none of them need to at present
[01:43] <StevenK> LFLayer, or is that too high?
[01:44] <wgrant> FunctionalLayer should be fine
[01:44] <wgrant> Don't even need DatabaseFunctionalLayer
[01:46] <StevenK> wgrant: http://pastebin.ubuntu.com/1595398/
[01:47] <wgrant> StevenK: Might want to extend the comment to state that we always want unicode, dammit.
[01:49] <StevenK> wgrant: http://pastebin.ubuntu.com/1595412/
[01:51] <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:52] <wgrant> Yep
[01:58] <wgrant> StevenK: I'll have ~3600 lines of diff for you soon :)
[01:58] <wgrant> Over 6 branches, though
[01:58] <StevenK> Whimper
[02:05] <wgrant> :)
[02:17] <wgrant> StevenK: So, do you understand what I'm doing?
[02:36] <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:37] <wgrant> StevenK: They're ready for you to review at your leisure
[02:55] <StevenK> wgrant: You never explained your dastardly plan
[03:04] <wgrant> StevenK: I've outlined it in the MP descs
[03:04] <wgrant> But can go into more detail if you desire
[03:05] <StevenK> wgrant: Yeah
[03:05] <wgrant> To which? :)
[03:06] <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:07] <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:08] <StevenK> -5-scorched-earth ? :-)
[03:29]  * StevenK hits line of 525 of -3.5 and vomits
[03:30] <wgrant> Mmm?
[03:30] <StevenK> 520	+ BinaryPackageBuild, Archive).find(
[03:30] <StevenK> 521	+ BinaryPackageBuild,
[03:30] <StevenK> 522	SQL(query))
[03:30] <StevenK> 523	resultset.order_by(
[03:31] <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:32] <StevenK> Which branch does DROP TABLE PackageBuild, then?
[03:32] <wgrant> -7 or so, because it's not quite so urgent :)
[03:33] <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:34] <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:36] <StevenK> wgrant: Up to -4
[03:39] <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:40] <StevenK> wgrant: I'm done, all approved. -3.5 has the most niggles
[03:40] <wgrant> Thanks
[03:54] <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:55] <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:56] <StevenK> That's hideous
[03:56] <wgrant> Quite!
[03:56] <StevenK> wgrant: Re-approved
[03:56] <wgrant> Thanks
[04:04] <StevenK> wgrant: Blink. My test failed on buildbot, but passes locally.
[04:04] <StevenK> 2.7 versus 2.6, at a guess?
[04:05] <wgrant> StevenK: Yeah
[04:06] <wgrant> 2.6 eats the whole character
[04:06] <wgrant> It assumes that the size indicated by the first byte is correct.
[04:07] <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:12] <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:13] <wgrant> With an XXX
[04:13] <StevenK> Right
[04:19] <StevenK> wgrant: http://pastebin.ubuntu.com/1595628/
[04:21] <wgrant> StevenK: Looks good
[04:26] <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:27] <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:28] <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:29] <StevenK> wgrant: Shall we deploy in about 70 minutes?
[04:29] <StevenK> Might be closer to 80, but eh
[04:30] <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:31] <StevenK> Yah
[04:31] <StevenK> I need to find another
[04:40] <StevenK> wgrant: How can I pull apart this checkwatches core ?
[04:41] <wgrant> You may not have much luck pygdbing it post-portem
[04:42] <wgrant> post-mortem
[04:43] <wgrant> Ideally we'd do it in-place
[04:45] <StevenK> On loganberry?
[04:46] <wgrant> Yes
[06:45] <wgrant> stub: Thanks. Those indices were indeed planned for a quick followup, but the DEFAULT thing slip my mind this time.
[07:16] <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:17] <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:18] <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:19] <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:20] <wgrant> The index only helps a lot if the column is relatively sparse
[07:32] <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:33] <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:34] <stub> Which is hundreds or thousands each gc run
[07:39] <wgrant> Oh yeah, that bit is a good point.
[07:39] <wgrant> Duh