[00:00] It's too easy for a factory bug or change to invalidate the test otherwise. [00:00] right [00:03] wgrant: new diff ready... [00:04] I guess I got the zcml stuff correct? I wasn't really sure about that... [00:04] Yep, that's right. [00:06] is securedutility something launchpad specific? I couldn't find it in http://docs.zope.org/zope.component/zcml.html [00:09] Yes, that's one of ours. [00:10] See SecuredUtilityDirective in lp.services.webapp.metazcml [00:10] But in general you shouldn't have to care about the details. [00:15] ok.. blindly copy + paste. check :D [00:17] wgrant: regarding the ugly list comprehension, were you suggesting it would be better to return a dict of dicts, rather than the list of ref dicts? [00:20] blr: Right, a list of dicts with a unique "name" field feels like it should be a dict. [00:21] (though that becomes more questionable if we ever batch that endpoint, that'll be an API break for other reasons anyway) [00:21] wgrant: yep, that would be better. Also, prepending 'refs/' will still be necessary in the view, but you're entirely right that it shouldn't in store. [00:21] Right, the view presumably keys on refs/, so it doesn't get that part of the path. [00:21] wgrant: well, when we need to address paging we can revisit it [00:21] Yep. [00:22] thanks [00:33] thomi: Approved with a couple of trivial fixes. [00:33] * thomi looks [00:33] thomi: Can you file an RT ticket to get your OpenPGP key added to the PQM keyring, so you can land things yourself? [00:33] Mail it to launchpad@ and I'll approve it. [00:34] wgrant: I file the rt by emailing launchpad@rt.canonical.com ? [00:34] yep [00:36] wgrant: filed [00:38] Got it, thanks. [00:41] wgrant: OK, I made those changes. Should to top-approve that MP, or would you like to look at it again? [00:41] I guess the PQM keyring won't get updated today [00:46] thomi: Top-approval doesn't do anything special for us today, as we don't use Tarmac. But I tend to use it as if we did. [00:46] Looks fine to me. I'll land it unless you have objections to your own branch. [00:46] no objections here :D [00:46] Good, I'd be worried. [00:47] heh [01:17] wgrant: I have another MP for you... the docs for zope_isinstance are rather lacking, so I'm not convinced that it's this easy... but the function is reasonably well tested, and the tests all pass, so... : https://code.launchpad.net/~thomir/launchpad/devel-fix-import-violations-buildfarmjob/+merge/251397 [01:19] thomi: zope_isinstance is just an isinstance that looks through security proxies. [01:19] Do the tests pass? I think that change is incorrect. [01:19] The structure here is a little bit confusing. [01:19] wgrant: the tests pass [01:20] Odd [01:20] I'm running lp.soyuz.browser.tests.test_builder_views.TestgetSpecificJobs [01:20] wgrant: I'm not sure what the word 'specific' means in that docstring [01:20] thomi: What about test_builder_views as a whole? [01:21] thomi: There are four types of build farm jobs. IBinaryPackageBuild, ISourcePackageRecipeBuild, ITranslationTemplatesBuild and ILiveFSBuild are all derivatives of IBuildFarmJob. [01:21] But, crucially, they are *specific* jobs. [01:22] They each have their own table named after them, and the data is primarily stored there. [01:22] wgrant: I'm running those tests now [01:22] But each specific job also references a non-specific row in the BuildFarmJob table, which is used for builder histories etc. that show all job types. [01:22] I'm still not sure what specific means in that context. [01:22] wgrant: those tests all pass as well :D [01:22] wgrant: just as a wee experiment, all of the refs in the linux git repo, serialized are 9.6k [01:22] BuildFarmJob is effectively an abstract base class. [01:22] Except it happens to actually exist for DB reasons. [01:24] thomi: So, the BuildFarmJob class is this non-specific DB table, which needs to be resolved to a concrete specific job for display. But IBuildFarmJob is implemented by both specific and non-specific classes, so your check changes the behaviour. [01:24] I was hoping a test would fail to prove that, but to preserve the behaviour you can check against IBuildFarmJobDB instead. [01:24] Only BuildFarmJob itself implements IBuildFarmJobDB. [01:24] (BuildFarmJob should probably be renamed to BuildFarmJobDB, but historical reasons) [01:25] Specific jobs have a non-specific BuildFarmJob. [01:25] Specific and non-specific jobs implement IBuildFarmJob. [01:25] Only non-specific jobs implement IBuildFarmJobDB. [01:26] so... that check should be 'IBuildFarmJob.providedBy(job) and not IBuildFarmJobDB.providedBy(job)' ? [01:26] zope_isinstance(job, BuildFarmJob) is only true for a non-specific job. [01:27] So I think it can be replaced with just "IBuildFarmJobDB.provideBy(job)" [01:27] oh, right. [01:28] hmmmm, I'll add a test for that failure case as well [01:28] This code and schema has been through three or four serious redesigns over the years, so the tests may well be dodgy. [01:29] yeah... some of these could be better named [01:29] * thomi gets out his yak razor [01:29] But basically the method is designed to take an arbitrary combination of non-specific and specific jobs, and turn them all into specific jobs. [01:29] Obviously for a specific job that transformation is a no-op. [01:30] It may be that the tests work because the non-specific and specific IDs match, which will happen if you only use one kind of job. [01:35] blr: We'll need to handle non-ASCII refs soon, but that can be a subsequent iteration. [01:37] wgrant: ugh, right. [01:41] I think #git exists primarily to slag off other dvcses, sadly. [01:42] Well, what else are they going to talk about? How good git's UI is? :) [01:44] wgrant: noooo... don't do it. [01:49] they are quite helpful with git intrisics support. [02:06] Evening xnox. [02:18] wgrant: if you have a moment: https://code.launchpad.net/~thomir/launchpad/devel-fix-import-violations-buildfarmjob/+merge/251397 [02:19] thomi: Done. [02:19] wgrant: I was tempted to rewrite that function, as I think i could be a lot clearer, but decided not to. [02:20] thanks... down to 5 violations now :D [02:20] thomi: You should feel free to rewrite things when it makes sense to. A lot of this code is old and things have evolved around it. [02:21] yeah... it would have been an aesthetic change only, and the tests aren't the best, so I figured it was best to leave it alone :D [02:21] Indeed. [02:22] thomi: Oh, I was about to land it for you, but blahdeblah has added you to the keyring. [02:22] oh! [02:22] thomi: 'bzr lp-land --no-qa' in the branch directory should do what you want. [02:22] Run it from your host, not inside the container, for keyring access. [02:22] oh, well that's easy then [02:22] You may need to grab bzr-pqm from the PPA, though. [02:23] And you'll need SMTP server config. [02:23] the container has access to gpg keyring too... [02:23] oh.. smtp thing will be "fun" [02:23] thomi: It needs launchpadlib keys too. [02:23] Which are in your gnome-keyring. [02:25] thomi: https://pastebin.canonical.com/126638/ has a superset of the relevant settings from my bzr configs. [02:26] how come no bzr-pqm package in utopic, but exists in saucy? [02:26] thomi: Because bzr and LP are probably the only two projects in the world that still use PQM. [02:26] oh ok [02:26] The saucy package should work, though. [02:27] the package in ppa:launchpad/ppa should as well I guess [02:28] Ah yes, I copied that up, didn't I. [02:35] * wgrant lunches. [02:58] thomi: Looks like you got lp-land working? [02:58] wgrant: maybe? The branch is still 'approved' [02:58] (also, you don't actually need to top-approve at all; lp-land will be happy as long as it has valid reviews) [02:58] thomi: Heh, check your email. [02:58] oh, right [02:58] You set from Merged to Approved, I guess you raced with the scanner. [02:58] I was about 10 seconds too early :D [02:58] ahh [02:59] wgrant: I wanted to ask you about the final few import violations [02:59] they're (almost) all where we're using load_related for (I think) database efficiency [02:59] Yup, that's what I thought they'd mostly be. [03:00] That's usually done with a DecoratedResultSet using pre_iter_hook. [03:00] Many methods in the model take an eager_load option, or a number of need_WHATEVER options to enable preloading of certain related objects. [03:01] Or some other sets have a preloadForWhatever method. [03:01] I'm not sure what the fix is for that - just move the code into a SomethingSet ? [03:01] for example, lp.soyuz.browser.archivesubscription line 334 (or thereabouts) [03:03] thomi: I'd fix getBySubscriberWithActiveToken. Depending on how many places it's used, you might just be able to push the load_related down into there without needing to make it optional. [03:03] Loading a handful of archives is relatively cheap, so it's not terrible to do it where it's not necessary. [03:03] But other types of preloading definitely want to be up to the caller to enable. [03:09] hmmm [03:09] Do you understand what all the preloading code is for? [03:10] It's to prevent the ORM from issuing a bazillion queries when accessing objects related to sets of other objects. [03:11] Each ArchiveSubscription obviously has a related Archive, and naive code looping through some subscriptions would issue a separate query for each subscription's Archive. [03:11] This is not good for performance. [03:11] load_related collects the IDs and does a single query for them all, to get them into Storm's cache cheaply. [03:11] somehwat unrelated, but presumably it'd only load the archive details when you accessed that part of the subscription object, right? [03:12] Right, code like "subscription.archive" pulls it in. [03:12] But it checks in Storm's cache (keyed on (class, primary key)) before making the query, which is why load_related prevents the extra query. [03:13] ok.. I think I just need to get my head around how this code gets moved around [03:13] You can probably just literally move the load_related call into getBySubscriberWithActiveToken, except that security proxies may ruin your day. [03:14] so getBySubscriberWithActiveToken would include the load_related call and throw away the result? I mean, it'd still be in the storm cache === micahg_ is now known as micahg [03:14] oh, except we use the result from load_related, but I guess if it's in the cache already we can look it up again and it should be cheap? [03:19] thomi: afk, but yes. once it's loaded the view can just loop through the subscriptions and get the archive out [03:19] awesome. There are some other things I can change in here as well. [03:20] yep, clean away [03:31] my brain is melting [03:31] :( [03:33] ugh. given that it's EOD for me, and I turn into a CI-team pumpkin tomorrow, I might back away sloooowly from this one :D [03:49] thomi: heh, no worries. you got a couple of good branches landed today :) [04:14] yeah [04:17] wgrant: Does http://lpqateam.canonical.com/qa-reports/deployment-stable.html limit the number of grey revisions? I expected to see two of mine at the bottom of the list, but I only see one... [04:17] qastaging may not have updated [04:17] Check what rev is running on qas [04:20] StevenK: doh - thanks, that should have been obvious :-/ [04:20] thomi: devpad has a log of what qas is doing, but I suspect WADL'ing [04:21] devpad.canonical.com gives me an empty directory listing :D [04:23] thomi: Via ssh [04:23] oh, gotchya [04:24] Somewhere under /srv [04:24] anyway, it's EOD for me, I'll care about it tomorrow if I have to :D [07:11] wgrant_: api-refs branch ready for re-review when you have a moment. === jamesh__ is now known as jamesh [11:47] wgrant: Oh, um, could you have a look at https://code.launchpad.net/~cjwatson/turnip/pastescript/+merge/251303 ? I just realised that I merged https://code.launchpad.net/~cjwatson/charms/trusty/turnip/api-server/+merge/251305 too early. [11:47] sigh Mondays [11:48] cjwatson: You should totally have self-reviewed that, but done. [11:48] Fair enough [11:48] Ta [15:30] Hey! [15:31] I'm experiencing some strange problems, I don't seem to be able to subscribe the 'landing-team-trackers' team to bug LP: #1425737 [15:31] Bug #1425737: Wizard freezes on blank screen after language

[15:31] ...aaaand nevermind! [15:32] Scratch that, it seems to have been some temprorary issue [15:32] I was worried since refreshing didn't help and I tried like 5 times [15:32] But now it's working again ;) [17:39] morning [17:53] Hi [17:54] I was wondering if we could have our team meeting today again this week, but neglected to ask people in my morning ... [17:54] * cjwatson grumbles at complex and not-entirely-predictable personal life conflicts but that's evening meetings for you [17:54] cjwatson: well I'm officially a CI person this week, so don't mind me :D [17:55] Aha [17:55] Splitter [17:55] and I doubt blr is up yet [17:55] yeah [18:04] I've spent half the day fighting (successfully, eventually) with juju, and the other half writing LP git xmlrpc tests [18:05] "xmlrpc" - that's a technology I haven't heard in a long time :D [18:06] ...reminds me of automating chyron devices [18:08] Yeah, well, we don't use it for talking from LP to turnip, but when talking from turnip to LP it's a lot easier than inventing a new endpoint stack [18:08] ahh, right [18:09] (Because we already have a private XML-RPC port that's used by several privileged services) === anthonyf is now known as Guest30629 === Guest30629 is now known as anthonyjf [22:46] cjwatson, blr: Today works for me. [22:50] morning wgrant :) [22:50] missing context, but I'm guessing that was our HO today rather than tomorrow? [22:52] blr: Right, Colin suggested it. [22:52] Though he may be gone now. [22:55] sure, I'm easy [22:59] Here [22:59] Aha, good morning. [22:59] Was just getting a bit of relaxing reading in beforehand [23:01] cjwatson: oh? [23:03] "Spin State", Chris Moriarty