[00:46] <StevenK> wgrant: The removal of self.log(self._output.output) is needed?
[00:46] <StevenK> (From lp-dev-utils)
[00:58] <wgrant> StevenK: I'm not sure what the real fix is, I just quickly deleted the line in the traceback to get ec2 to work.
[01:00] <StevenK> I have another change to make, so I'll ignore that removal for now
[01:06] <StevenK> [2013-05-08 00:58:14,724: INFO/PoolWorker-3] Job resulted in OOPS: OOPS-f707f73e2b097961847648306d155654
[01:06] <StevenK> InternalError: current transaction is aborted, commands ignored until end of transaction block
[01:07] <wgrant> There was an earlier OOPS
[01:07] <StevenK> Not that I can see in the log
[01:11] <StevenK> wgrant: So, unscan-branch or delete and repush?
[01:49] <wgrant> flacoste: Are you using Chromium to get those librarian 404s?
[02:23] <wgrant> StevenK: What's the new DB MP?
[02:26] <StevenK> I had to delete them because sync-pipeline completly lost its mind
[02:28] <StevenK> wgrant: Right, all six MPs are up, all with diffs.
[02:28] <wgrant> StevenK: Only six? I'm disappointed.
[02:28] <wgrant> Will look soon
[03:15] <wgrant> StevenK: 350	+ def findBranchMergeProposalIDs(self):
[03:15] <wgrant> 351	+ return self.store.find(
[03:15] <wgrant> 352	+ (BranchMergeProposal.id),
[03:15] <wgrant> That doesn't do what you think it does
[03:15] <wgrant> () is an empty tuple, but (foo) is just foo
[03:36] <StevenK> wgrant: The garbo job actually works, though ...
[03:38] <wgrant> StevenK: But the code still doesn't mean what you intended, so probably best to drop the parens.
[03:38] <StevenK> wgrant: Right.
[03:39] <StevenK> wgrant: Killing yui_3.3.0.zip from sourcedeps, should yui_3.5.1.zip also meet its demise?
[03:39] <wgrant> StevenK: Might as well
[03:46] <StevenK> wgrant: Fix for http://pastebin.ubuntu.com/5643581/ landing/landed.
[03:55] <StevenK> wgrant: Your comment about removing merge_proposal_id is on the right MP?
[03:58] <StevenK> Oh
[03:58] <StevenK> Why?
[03:59] <wgrant> StevenK: Why not?
[03:59] <wgrant> It was added just for garbo
[03:59] <wgrant> And now garbo is gone
[03:59] <StevenK> It was not.
[03:59] <wgrant> Wasn't it?
[03:59] <StevenK> You should read switch-bmp-to-previewdiff-merge_proposal carefully.
[04:00] <wgrant> Ah, so you're leaving it for a reference that only appears in a later branch?
[04:00] <StevenK> Pretty much
[04:00] <wgrant> That's OK, then.
[04:00] <StevenK> We really can't use it in the code until it's populated
[04:00] <StevenK> 371 + @property
[04:00] <StevenK> 372 def branch_merge_proposal(self):
[04:00] <StevenK> Why does this still exist?
[04:01] <StevenK> wgrant: Because it's exported :-(
[04:01] <wgrant> StevenK: Hm, can you just call the property that, then?
[04:01] <wgrant> Rather than having merge_proposal separate
[04:01] <wgrant> Just call it branch_merge_proposal
[04:02] <StevenK> So the merge_diff madness continues
[04:02] <wgrant> Hm?
[04:02] <StevenK> merge_proposal_id / branch_merge_proposal probably isn't going to make you like me
[04:03] <wgrant> No, that's illegal.
[04:03] <StevenK> Which means we have branch_merge_proposal = Int(name='merge_proposal')
[04:03] <wgrant> Or you rename the DB column
[04:03] <wgrant> This sort of thing is why you don't land DB patches early. Makes them easy to revise in context :)
[04:04]  * StevenK twitches at having to rename it in six branches
[04:04] <StevenK> Well, five
[04:17] <wgrant> StevenK: Also, have you considered pruning?
[04:17] <wgrant> Until we have pruning, bmp.preview_diffs may grow without bound
[04:18] <StevenK> I was thinking about that yesterday, but I'm not sure about the prune condition, since PreviewDiff.branch_merge_proposal is NOT NULL
[04:18] <wgrant> We want to prune unreferenced ones. At the moment that probably just means everything except the latest.
[04:18] <wgrant> But that will change once we start tying comments to a particular diff version
[05:38] <StevenK> wgrant: Compound index?
[05:42] <wgrant> StevenK: So we can efficiently answer "what is the current diff for this merge proposal?"
[05:44] <StevenK> wgrant: Sure, I'm not sure what that index looks like, and if we need the current index in the patch
[05:44] <wgrant> StevenK: (branch_merge_proposal, date_created)
[05:45] <wgrant> (branch_merge_proposal) lets me quickly identify the diffs for the merge proposal, but without the additional sort of date_created I'd have to examine all of them to find the newest.
[05:58] <stub> wgrant: how many diffs are you expecting to be attached to a MP?
[05:59] <stub> If it is normally one or two and we never have thousands, I don't think the extra maintenance of the index is worth it.
[06:00] <wgrant> stub: date_created will never change, and the index will always be appended to, so is that significant?
[06:01] <wgrant> It should rarely be more than a dozen, but in pathological cases it may be.
[06:01] <wgrant> And I don't see the expense of the additional index column as being particularly significant.
[06:02] <stub> An extra index to keep hot, extra writes on insert, fatter index vs thin index and extra row loading...
[06:02] <stub> And the overhead of all the extra typing in the db patch ;)
[06:02] <wgrant> It's not an extra index, just a wider one.
[06:03] <stub> oh right, new column.
[06:03] <wgrant> We need one with branch_merge_proposal at the start either way
[06:04] <stub> yup. I'm not fussed one way or t'other.
[06:05] <wgrant> If it were a larger table then I would be concerned about the wider index, but I don't think it's an issue here.
[06:29] <wgrant> StevenK: https://code.launchpad.net/~wgrant/launchpad/dont-overrideFromAncestry/+merge/162933
[06:42] <StevenK> wgrant: r=me
[06:42] <wgrant> StevenK: Thanks.
[06:55] <StevenK> wgrant: Glance at the six again?
[23:42] <StevenK> wgrant: When you're done with <redacted>, can you peer at the six MPs again?
[23:45] <wgrant> StevenK: Sure