[00:04] https://code.launchpad.net/~thumper/launchpad/epic-email-bug-description/+merge/51977 [00:04] wallyworld: abentley won't still be here === thumper changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | firefighting: - | On call reviewer: - | https://code.launchpad.net/launchpad-project/+activereviews [00:04] wallyworld: not really :-) [00:05] abentley: i was hoping for a quick +1 on the recipe build perf mp after i fixed the test :-) [00:05] wgrant: have a look at the same urls in https://bugs.launchpad.net/launchpad/+bug/711064 [00:05] <_mup_> Bug #711064: POFile:+translate timeouts < https://launchpad.net/bugs/711064 > [00:05] wgrant: (click through I mean) - quite a bit snappier :) [00:06] abentley: i take your point about deleted recipes but the page was only designed to show builds with recipes and this mp is a perf tweak. extending what the page is designed to do can be done separately :-) [00:07] wallyworld: It doesn't matter what the page is designed to do, because this is model code. [00:07] the page will never show builds without recipes due to how the query is structured [00:07] abentley: I thought it was view vode [00:07] wallyworld: anyone can see it and reasonably assume that it is safe to call. [00:07] ie select stuff from recipe join build ..... [00:07] lifeless: no, model. [00:08] the recipe will never be None on the page [00:08] wallyworld: It does not matter whether the recipe will ever be None on the page. [00:08] so it is always safe to call [00:08] sorry, i'm missing something i think [00:09] wallyworld: it is not safe to call if the recipe is None, and there's no reason to believe that it will be non-None for an arbitrary caller. [00:09] but it isn't a view off of a recipe [00:09] it is a view off ILanchpadRoot which runs a query and displays some data [00:09] lifeless: Still 4s :( [00:10] wgrant: down from ... 12 [00:10] and that data will only ever contain valid recipes and associated build info [00:10] wallyworld: It looks very much like you've added two properties to RecipeBuild. [00:10] yes, and the recipe there will never be None [00:10] wallyworld: is that not the case? [00:10] wallyworld: Yes, it will sometimes be none. [00:10] how? [00:11] the query is guaranteed to return records with recipes [00:11] wallyworld: What query? The query is not connected to RecipeBuild. [00:11] lifeless: :( [00:12] wallyworld: It is fully possible for an arbitrary caller to have a RecipeBuild with a None recipe, and to access recipe_name. [00:12] abentley: the query which is used to implement findCompletedDailyBuilds() [00:12] but i guess if they are paging through the results, the recipe could be deleted after the page was first rendered [00:12] wallyworld: You have no guarantee that the RecipeBuild was returned by that methode. [00:13] wallyworld: Arbitrary code in Launchpad can access RecipeBuilds and then access recipe_name. [00:13] abentley: the RecipeBuildRecord used in the collection backing the view is constrcuted directly from the result set [00:14] the RecipeBuildRecord is a named tuple [00:14] with data needed by the view shoved into it [00:14] wallyworld: so you're saying that even though this code lives in model, it's actually view code? [00:15] well, its code which queries the model and returns some data [00:15] the data is currently used by a view but it could be used elsewhere too [00:16] the main class is RecipeBuildRecordSet [00:16] there's also a BugTaskSet for example that is model code [00:17] the rendering of the data is done in the view layer [00:20] wallyworld: so my main source of confusion is that I thought we were talking about SourcePackageRecipeBuild, not RecipeBuildRecord, which I don't think I've seen before. [00:21] ah ok. the number of times i've be confused between SourcePackageRecipeXXXX, SourcePackageReleaseXXX is too many to mention. so many similar names [00:21] etc [00:22] wallyworld: well, it's complicated by the fact that SourcePackageRecipeBuild and RecipeBuildRecord seem to refer to exactly the same thing. [00:22] not exactly :-) [00:24] wallyworld: How not? [00:24] the later is akin to a denormalised chunk of data, the other is a straight model object [00:25] wallyworld: but they both refer to the same thing, and you can query either to get the same data. [00:25] model object in this case = persistent database object perhaps [00:27] i would have to take a closer look, but can SourcePackageRecipeBuild directly provide the SourcePackageName, the Archive, the lastest build time etc? [00:28] wallyworld: You can get those from SPRB.source_package_release. [00:28] but not without navigating the object model [00:28] onely one page of getPublishedSources references to update [00:28] >< [00:29] SourcePackageRecipeBuild is designed to package all required data into a readonly single object, a snapshot of the system state if you like, for easy access [00:29] to fit the use case [00:29] i meant RecipeBuiildRecord [00:29] not SourcePackageRecipeBuild [00:30] wallyworld: I get that you have easier, more efficient access to that data. [00:30] wallyworld: I still think the RecipeBuildRecord refers primarily to a SourcePackageRecipeBuild. [00:31] wallyworld: And the rest is just precaching. [00:31] and it comes straight off the query so it can be got with fewer object instantiations, so more efficient [00:31] wgrant: another in _collectLatestPublishedSources [00:31] wallyworld: I wasn't talking about whether it was efficient or direct. [00:32] wallyworld: I was talking about what it referred to. [00:32] ah ok [00:32] so given the scope of the mp, ie a perf tweak, you're happy to +1? :-) [00:33] wallyworld: I will reluctantly +1 it. I don't see why you're leaving booby-traps for people who extend it later. [00:34] wallyworld: When the fix is a two-line conditional. [00:34] i don't get what the booby traps are. it's not that uncommon not to pass around entire persistent objects / model objects and instead construct objects more suited to a given use case? [00:35] but the conditional is not required [00:35] i can add it though [00:36] wallyworld: AIUI you're saying that this memo object is never constructed without a recipe [00:36] Project db-devel build #413: STILL FAILING in 4 hr 50 min: https://hudson.wedontsleep.org/job/db-devel/413/ [00:36] yes [00:36] wallyworld: the booby-trap is that if someone ever constructs a RecipeBuildRecord whose SourcePackageRecipeBuild.recipe is None, then recipe_name will raise an AttributeError. [00:36] wallyworld: and that someone constructing it without a recipe would be changing its contract, so there will be other changes they have to make. [00:36] since it is constructed in the resultset decorator [00:36] StevenK: If you see any bug comment spam, you can now hide it yourself. [00:37] wallyworld: I have no particular opinion here, just trying to make sure I understand [00:37] the RecipeBuildRecords are not meant to be constructed with null recipes [00:37] wallyworld: If that is the contract, then you could put it in the constructor. [00:37] yes, i could an an assert [00:38] add [00:38] wgrant: I didn't think that rolled out yet [00:38] hopefully namedtuples let me do that [00:38] wallyworld: that would make sense to me. From looking at that object, I see nothing to suggest such a contract. [00:38] fair call [00:38] StevenK: Just in the last hour or so. [00:39] i think doing that will remove a lot of the confusion [00:39] i guess from my perspective, i never expected one would want or need to construct the object without a recipe [00:40] lifeless: you had a question? [00:40] wallyworld: I expect we'll want to construct it with arbitrary SourcePackageRecipeBuilds before long. [00:40] wallyworld: and that would include ones without a recipe. [00:40] wallyworld: Looking at production data, there are many SourcePackageRecipeBuilds where recipe is None, because the user has deleted the recipe. [00:41] abentley: at the moment, it's only constrcuted from the resultset decorator, but that could change [00:41] wallyworld: no, I asked questions to establish a clear picture of the debate [00:41] wallyworld: right. [00:41] StevenK: but those records will never be returned due to how the query is constructed [00:41] lifeless: oh, i didn't see your questions [00:42] wallyworld: you answered them :) [00:42] wallyworld: I am +0 on adding *another* class for Recipe builds [00:42] This strikes me as needless duplication [00:42] StevenK: I don't think anyone is suggesting that. [00:42] lifeless: ok. hopefully you were satisfied with the answers [00:43] abentley: Then I apologise, since I've obviously read what wallyworld is doing wrong. [00:43] StevenK: wallyworld is just extending the existing RecipeBuildRecord class. [00:43] wgrant: Can haz howto? [00:43] StevenK: it's not duplication since it's not a model object - it's akin to a DTO to package some data needed for a given use case [00:43] StevenK: lp.bugs[1234].setCommentVisiblity(comment_number=2, visible=False) [00:43] With Visibility spelt properly. [00:43] wgrant: I can't do it via the web UI? [00:43] StevenK: No UI. [00:44] But it's a trivial API script. [00:44] Bah, fail. [00:44] You'd think the API script would be on the wiki somewhere ... [00:45] abentley: thanks for the input. i think we managed to reach an acceptable solution once i add the asserts, or maybe i'll just cater for null recipe if we think that's a future possibility, but that will mean changing the page template too perhaps [00:45] StevenK: It is two lines... [00:46] StevenK: I'm not sure that anybody has bothered to publish it. [00:46] I can has OCR? https://code.launchpad.net/~lifeless/launchpad/bug-727560/+merge/51989 [00:51] lifeless: seems there's no OCR but i can have a go if you like [00:52] or you may prefer a proper review first up [00:54] wallyworld: feel free; we can tap thumper to mentor ;) [00:54] ? [00:55] lifeless: https://code.launchpad.net/~thumper/launchpad/epic-email-bug-description/+merge/51977 [00:55] thumper: I have a moderate size, 90% mechanical because of stormification branch needing review [00:55] thumper: on yours now [00:56] typo [00:56] lots *of* text to add [00:57] thumper: i was going to have a go since no one was around but if you want it.... [00:57] wgrant: want to review thumpers, then I'll mentor? [00:57] wgrant: I see 2 things. [00:57] wallyworld: no, you take it [00:57] thumper: great. i'd like the practice [00:58] lifeless: Sure, looking. [00:59] * wallyworld quickly finishes daily recipe build perf fix first so ec2 can chew on it [01:01] :-((( [01:01] wgrant: Right, all done. Six comments hiden [01:01] *hidden [01:01] StevenK: Thanks. [01:01] time out proposing a review [01:01] Whee [01:02] thumper: Was it scanning? [01:02] possibly [01:06] https://code.launchpad.net/~thumper/launchpad/process-email-oops-on-person-adaptation/+merge/51992 [01:09] thumper, lifeless: Reviewed the email size thing. [01:09] wgrant: ta [01:13] wgrant: thumper: mentored [01:14] wgrant: want to mentor https://code.launchpad.net/~thumper/launchpad/process-email-oops-on-person-adaptation/+merge/51992 ? [01:14] lifeless: ok [01:14] d'uh [01:15] me didn't read [01:19] lifeless: Looking. [01:21] lifeless: Done. [01:29] thumper: second one reviewed. [01:31] Thanks. [01:33] * thumper relocates [01:34] lifeless: wondering why candidates[0] and not candidates.first() [01:34] or are they the same thing in terms of efficiency [01:35] wallyworld: .first() is clearer, they should be equivalent for efficiency. Without reading the code I am not /sure/ that .first() raises IndexError, so I left some [0] around for that case. [01:35] wallyworld: also some tests were going to be less clear with .first() so I left those alone too [01:36] lifeless: ok. i assume that because only Archive.getPublishedSources() was change and not the others, the the 2 resultset types are interchangable? [01:36] forgive my storm ignorance [01:36] wallyworld: which 2 resultset types? [01:36] wallyworld: (there are, uhm, 4 or so around) [01:36] wallyworld: and interchangable /where/ [01:36] SQLObjectResultSet and DecorateResultSet [01:37] they are not interchangable and not compatible [01:37] thats why I stormified getPublishedSources so that I could use DRS [01:40] ok. i saw a few getPublishedSources() methods and initially thought they were off the one interface but not so [01:40] so my question doesn't make sense anymore [01:42] its entirely possible I have some things in this that ec2 will highlight as faults [01:42] but I think I identified the types correctly [01:43] yeah, i suspect i was seeing an issue that's not there. hard to tell sometimes from looking at diffs [01:45] storm sure seems better than what it replaced, looking at the diffs - less string literals and more object based approach [01:54] how do you make a scanned branch in a test ? [02:04] \o/ [02:04] * 3472 Exceptions [02:04] flat cunt for timeouts [02:04] *count* [02:07] lifeless: keep it clean :-) [02:08] nice drop in bugtask:+index timeouts [02:09] We should be below 2000 excepts per day tomorrow. [02:10] wgrant: what was the peak value before the tuning work started in earnest? [02:10] wallyworld: I don't really know. But a few days ago it was 23000. [02:10] !!!! [02:10] 16000 of those were that loggerhead thing, though. [02:10] So it's sort of cheating. [02:11] still 7000 (or anything with that many zeros at the end) is too many [02:11] except if its your bank balance - then too many zeros are never enough :-) [02:12] * wallyworld needs lunch [02:13] thumper: hey [02:14] wgrant: Test failures with my TAL branch, do agree with http://pastebin.ubuntu.com/574779/ [02:16] StevenK: Looks reasonable. [02:16] And the tests pass, so I'll toss it at ec2 again [02:16] ec2 is only 3.5 hours now. [02:16] It's nice. [02:17] Heh, yes [02:30] lifeless: yus? [02:31] thumper: I have a problem [02:31] ok [02:31] thumper: I https://bugs.launchpad.net/launchpad/+bug/722956 [02:31] <_mup_> Bug #722956: BranchSet:CollectionResource#branches timeouts

< https://launchpad.net/bugs/722956 > [02:31] thumper: I had a branch that eager loads that [02:31] thumper: but it failed on qastaging [02:31] thumper: i fixed a typo, but then it failed again with a TypeError doing some adaption [02:31] thumper: IIRC the error properly, it looked like a source package series branch [02:32] thumper: but I don't know how to set one up in the test suite [02:32] [02:32] def test_renders_with_source_package_branch(self): [02:32] source_package = self.factory.makeSourcePackage() [02:32] branch = self.factory.makeBranch(sourcepackage=source_package) [02:32] branch.updateScannedDetails(None, 0) [02:32] logout() [02:32] lp = launchpadlib_for("test") [02:32] list(lp.branches) [02:32] is what I have [02:32] but I suspect its not a 'source package series branch' [02:32] because the url is a long one with ~user in it [02:33] 3 TypeError: ('Could not adapt', , ) [02:33] lifeless: a package branch is one whose target is a distro series source package [02:33] thumper: yup, I've got that far [02:34] thumper: then there is the *SeriesSourcePackageBranch* which is the blessed ones [02:34] lifeless: also, use factory.makePackageBranch [02:34] right [02:34] how do I get from what I have to the blessed one [02:35] off the top of my head... [02:35] something like [02:35] ICanHasLinkedBranch(branch.target).branch = branch [02:36] ICanHasLinkedBranch(naked_product).setBranch(branch) [02:36] copied from a test [02:37] lifeless: lib/lp/code/model/tests/test_branch.py:803: [02:38] lifeless: is one way [02:39] lib/lp/code/model/tests/test_linkedbranch.py [02:39] looking, thanks === Ursinha is now known as Ursinha-afk [02:43] thumper: thanks! [02:43] thumper: have reproduced the boom [02:44] oh, no, I haven't [02:44] I made the test do it instead >< [02:51] thumper: thanks, *now* I have the failure reproduced. [02:51] cook [02:51] cool [02:52] its a bit messy - I copied all of lib/lp/code/model/tests/test_branch.py:803: basically, but thats tolerable for now [03:01] ok, fixed [03:01] lp-land time. [03:04] wgrant: you were right, I was checking the entire email message [03:04] wgrant: I've fixed it, and pushing now [03:04] thumper: Great, will look in a sec. [03:07] thumper: can you mentor https://code.launchpad.net/~lifeless/launchpad/bug-727560/+merge/51989 please? [03:11] thumper: Why do you need ZopelessAppServerLayer? [03:11] That actually starts up an appserver process, IIRC. [03:11] wgrant: as opposed to? [03:11] the content gets stored in the librarian [03:11] Plain LaunchpadZopelessLayer. [03:11] the email bits that is [03:11] the script runs in a zopeless environment [03:12] so the test should mirror that IMO [03:12] LaunchpadZopelessLayer mirrors that [03:12] to be honest, I copied the layer from the TestCodeHandler test [03:12] the appserver layers are IMNSHO hideous [03:12] I'm not entirely sure [03:12] AppServerLayer is a bug. [03:12] I could try it with the LaunchpadZopelessLayer [03:12] thumper: I suggest you try LaunchpadZopelssLayer [03:12] jinx [03:12] LaunchpadZopelessLayer provides a librarian. [03:15] so... what does the ZopelessAppServerLayer provide? [03:15] An appserver in a separate process. [03:15] a running zope instance in a subprocess [03:15] it was added for mailman [03:16] and should be nuked from orbit along with the way we intgrate mailman [03:16] Most stuff using it is probably a bug. [03:16] hmm... [03:16] needed for the code handler :( [03:16] Oh? [03:16] TestCodeHandlerProcessMergeDirective [03:16] don't ask me why [03:16] I'm not sure [03:17] It doesn't need it. [03:17] It may use it. [03:17] But it doesn't need it. [03:17] if fails without it [03:17] and passes with it [03:17] That may just mean that the test is broken. [03:18] :) [03:18] changed the layer [03:18] and pushed [03:18] Thanks. [03:22] lifeless: I spoke to Barry about Mailman 3 at the distro sprint. He said he's close, but there's two things that need fixing. [03:22] StevenK: I know [03:22] StevenK: its one of those things that will happen in 2013 [03:22] * lifeless sadly isn't kidding [03:24] That makes me sad. :-( [03:26] its not on our todo list for -ages- [03:26] and barry has not got time to do a major overhaul in lp space [03:26] good intentions aside [03:26] lifeless: in your mp, why the need for 'distroseriesID = Attribute("DB ID for distroseries.")'. i thought storm did that automagically? [03:27] wallyworld: thats on the interface. [03:27] wallyworld: if its not on the interface security proxies kick you out [03:27] ah, yes. thanks [03:28] lifeless: Well, he also said the two things are broken in core, which means they might get fixed soon. :-) === Ursinha-afk is now known as Ursinha [03:31] thumper: you need to click 'approve' as well [03:31] lifeless: it isn't approved [03:31] lifeless: wallyworld said needs info [03:32] thumper: ian approved it [03:32] * thumper looks again [03:32] I bet he added comments while I had it open [03:32] where's our long-poll [03:32] and optimistic locking :-) [03:35] thumper: thanks! [03:39] Is it just me or are a bunch of Canonical's websites down? [03:40] Such as? [03:40] StevenK: Well Everything. I can't access the irc server, ubuntu.com, canonical.com etc. [03:40] All good for me. [03:40] What does traceroute say? [03:41] And me. [03:43] Traceroute errored after one of internode's servers, but I seem to be connecting again now [03:44] I suspect a route failed over or so [03:46] oh well as long as it was just me [03:48] still flaky though [03:50] huwshimi: I've just started getting ~50% packet loss in the US. [03:50] To which part of the US? [03:50] My route to Fremont is fine. [03:50] wgant: which isp are you with? [03:51] StevenK: Within L3 in San Jose. [03:51] huwshimi: Optus. [03:51] StevenK: My route to Fremont is fine too. [03:51] Doesn't go through L3. [03:51] Oddly. [03:52] I hit Exetel, Alter, and then my server [03:53] I have just one (anonymous) hop between Optus and HE. [03:56] I have one anonymous hop, but it's Telstra's DSL bridges that they like hiding [04:08] I'm well and truely sick of hakcing on BugTask:+index [04:08] won't another page please start timing out massively. I need a change. [04:10] wallyworld: I thought you were nuking '155 BugWatchUpdateWarning' [04:10] ExternalBugtracker for BugTrackerType 'GOOGLE_CODE' is not known [04:10] lifeless: i've no idea what you are referring to :-) [04:10] bach [04:10] wgrant: ^ [04:10] lifeless: That was me. [04:10] But it requires checkwatches.txt to be redone. [04:11] It is the hardest OOPS to get rid of. [04:11] So I did the others first. [04:11] wgrant: the doctest ? [04:11] i can see how you keep getting us confused, but for future reference, i'm the good looking one [04:11] lifeless: Yes. [04:11] wallyworld: its all about the tabs [04:11] wallyworld: if I fumble the g, it tab completes to you [04:11] wgrant: my sympathies [04:11] yeah i know, i was just messing with you [04:11] wgrant: if you need a tactical nuke, let me know [04:12] lifeless: We can probably drop that bit of the doctest, but it may be the only test that running the script logs OOPSes properly. [04:12] I will hopefully get back to that next week. [04:12] There are what.. 300-400 of them a day? [04:12] yeah 10% of remaining daily exceptions [04:13] Note that 1500 of yesterday's exceptions are already fixed. [04:13] So we are down to ~2000. [04:13] wgrant: -lovely- [04:13] So it's more! [04:19] hmm [04:19] I find the new style strains my eyes :( [04:19] I think body and comment text is slightly too small. [04:19] they are to faint [04:19] I've had a couple of friends say the same thing. [04:19] too [04:21] wgrant: hmm [04:21] Hmm? [04:22] wgrant: reckon you could run dogfood with sql backtraces enabled [04:22] and find where the milestone lookups in https://lp-oops.canonical.com/oops.py/?oopsid=1888C48#repeatedstatements are being triggered ? [04:22] lifeless: Sure. [04:22] ditto the distribution one [04:22] thats another second if we fix both of those. [04:23] https://bugs.launchpad.net/launchpad/+bug/724033 to record what you find; I'll work up fixes. [04:23] <_mup_> Bug #724033: BugTask:+index timeouts - distributions and milestones being late evaluation loaded - repeatedly - on bug 230350 < https://launchpad.net/bugs/724033 > [04:23] lifeless: Could you run http://paste.ubuntu.com/574807/ on staging? [04:23] I'm merging the tracebacks branch. [04:23] qas or s? [04:24] wgrant: ^ [04:24] lifeless: Whichever has the newer DB. [04:24] $meh [04:24] Doesn't really matter. [04:24] its running [04:24] Thanks. [04:25] Shouldn't take too long. [04:25] you want the output, or timing ? [04:25] Output please. [04:25] Shouldn't be much. [04:25] I hope. [04:25] will tell you when it finishes [04:26]