[01:14] wgrant: the recent apa model code - i haven't looked in detail, but it doesn't seem to update the access_policies or access_grants columns of bug task flat? [01:15] wallyworld_: BugTaskFlat will continue to be trigger-maintained in perpetuity, as it's still not feasible to hook or even identify all the relevant app code. [01:16] wgrant: so we still need to separate out the btf triggers then [01:16] wallyworld_: They're deliberately already separate [01:16] bug_maintain_bug_summary_trigger AFTER DELETE OR UPDATE ON bug FOR EACH ROW EXECUTE PROCEDURE bug_maintain_bug_summary() [01:16] bug_mirror_legacy_access_t AFTER INSERT OR UPDATE ON bug FOR EACH ROW EXECUTE PROCEDURE bug_mirror_legacy_access_trig() [01:17] hmmm. i thought the btf code and apa stuff just added to the model was in the same trigger [01:17] Nope. [01:17] BTF is entirely separate. [01:17] accessartifactgrant_maintain_bugtaskflat_trigger AFTER INSERT OR DELETE OR UPDATE ON accessartifactgrant FOR EACH ROW EXECUTE PROCEDURE accessartifact_maintain_bugtaskflat_trig() [01:18] accesspolicyartifact_maintain_bugtaskflat_trigger AFTER INSERT OR DELETE OR UPDATE ON accesspolicyartifact FOR EACH ROW EXECUTE PROCEDURE accessartifact_maintain_bugtaskflat_trig() [01:18] etc. [01:18] ok, i'll look elsewhere to see why btf is not being maintained with legacy triggers disabled [01:18] What sort of operation are you performing? [01:18] a search [01:18] And what does the failure look like? [01:18] And which triggers are you disabling? [01:19] not a failure - a former grantee is still being returned [01:19] after calling transition to target [01:19] transitionToTarget won't change grantees. [01:19] It might change policies. [01:20] am disabling bugtask_mirror_legacy_access_t etc [01:20] yes, an a person with a olicy grant should then no longer see the bug [01:20] Do you have a diff? [01:20] Right. [01:20] i'll look into it a bit to see what's happening [01:21] transitionToTarget will call updateAccessPolicyArtifacts, which will remove the row from AccessPolicyArtifact, which will cause the trigger to update BugTaskFlat. [01:21] that's what i was expecting [01:26] wgrant, wallyworld_: https://code.launchpad.net/~stevenk/launchpad/set-spph-packageupload/+merge/108513 [01:28] ugh, soyuz, pass [01:28] Hahaha [01:28] wallyworld_: You said you want to learn it :-P [01:28] not today though [01:29] wallyworld_: It's only a 17 line query, that's *tiny* :-P [01:29] StevenK: I'd check that PU.changesfile IS NOT NULL to make sure it's a real upload. [01:29] Also perhaps check that it's the earliest SPPH for that SPR [01:29] wgrant: Do we care? We'd get a hold of some delayed copies PUs too, but I don't think it matters [01:30] StevenK: We want to be conservative here. We can easily detect data that we haven't filled in, but it's harder to detect and correct data that exists but is wrong. [01:31] Hmm, I'm not sure how to check it's the earliest SPPH [01:32] So I'd add both the checks I mentioned. changesfile IS NOT NULL ensures it's a real upload, and restricting to the first SPPH prevents it from incorrectly matching post-upload overrides. [01:32] spph.id = (SELECT MIN(spph2.id) FROM sourcepackagepublishinghistory spph2 WHERE spph2.archive = spph.archive AND spph2.sourcepackagerelease = spph.sourcepackagerelease) or so [01:36] wgrant: Hmm, make sense. Now there's this Storm thing. Let me see. [01:36] Easy in Storm. [01:40] wgrant: Hm, really? My 5 minutes of playing around has yielded nothing. [01:41] spph2 = ClassAlias(SourcePackagePublishingHistory) [01:41] SourcePackagePublishingHistory.id == Select(Min(spph2.id), tables=spph2, where=And(spph2.sourcepackagereleaseID == SourcePackagePublishingHistory.sourcepackagereleaseID, spph2.archiveID == SourcePackagePublishingHistory.archiveID)) [01:41] or so [01:42] Oh Select() [01:42] That is how one does a subselect. [01:42] I was trying to do it directly :-( [01:43] (a quick query here shows that there are about 294 publications in oneiric that that will exclude, which seems a little high, but perhaps it's correct) [01:45] Ah, forgot to match section [01:45] 31, that's more plausible [01:49] StevenK: dogfood confirms that I am correct. [01:49] There are 30 cases in oneiric where the MIN(spph2.id) bit is required. [01:49] eg https://launchpad.net/ubuntu/+source/im-config/+publishinghistory [01:50] Note how it started in universe, was promoted, and then demoted again [01:50] Your query would set both universe publications to the PU [01:50] Which is a lie. [01:52] wgrant: Yeah. Writing a test for it [01:53] https://pastebin.canonical.com/67361/ [01:53] Note that they all have 2 or 3 publications with the same overrides, except for ppl which was a delayed copy (because I omitted the pu.changesfile IS NOT NULL clause) [01:57] launchpad_dogfood=# SELECT COUNT(*) FROM packageupload pu JOIN packageuploadsource pus ON pus.packageupload = pu.id JOIN sourcepackagerelease spr ON spr.id = pus.sourcepackagerelease WHERE pu.changesfile IS NOT NULL AND pu.archive <> spr.upload_archive; [01:58] count [01:58] ------- [01:58] So my other assumption is correct too [01:58] 0 [01:58] (1 row) [01:58] (PU.changesfile == original upload) [01:58] (PU.changesfile IS NOT NULL => original upload) [01:58] oh right, you guys are working today. Doh! ;) [02:01] wgrant: pu.changesfiles IS NOT NULL will miss PCJs, won't it? [02:01] StevenK: Yes. We can't match PCJs this way. [02:01] We'd have to match the PCJ to match the PCJ. [02:31] wgrant: http://pastebin.ubuntu.com/1022402/ [02:32] Not sure why bzr di things I replaced one of the methods wholesale. :-( [02:32] s/g/k/ [02:32] StevenK: Let's see [02:32] wallyworld_: Any luck? [02:33] wgrant: The tests all pass, too, so I don't feel like a complete idiot after my subselect fail. [02:33] StevenK: Possibly better to call spph2 matching_spph or something like that. [02:33] + def test_PopulateSPPHPU_no_changesfile(self): [02:33] + # PopulateSourcePackagePublishingHistoryPackageUpload will not [02:33] + # set a SPPH's packageupload to a non-original upload. [02:33] that comment could be a bit clearer [02:34] And I suspect that common bits of the tests can be factored out [02:34] But otherwise looks good. [02:34] Hmmm, you're right [02:34] Let me do that [02:34] I'd like comments in the non-test code too, though [02:34] It's currently not exactly obvious to anyone but me why those two bits are there :) [02:35] wgrant: no. logging the sql shows an insert and then a delete from AccessPolicyArtifact, but a subsequent search selecting from btf testing (BugTaskFlat.access_policies)&&(SELECT ARRAY_AGG(AccessPolicyGrant.policy) returns true [02:35] Just Curious. Is there a particular reason that is a comment instead of a docstring? [02:35] Not sure. It's Launchpad convention to use a comment. [02:36] wallyworld_: odd. What if you break in the test or commit and inspect the tables? [02:36] Do you guys use nose? [02:36] wgrant: yeah, looking into that now [02:36] cody-somerville: We do not. [02:36] wallyworld_: Otherwise throw me a branch/diff and I'll have a poke. [02:38] ok, will do after i look into things a bit more [02:42] wgrant: http://pastebin.ubuntu.com/1022422/ [02:43] StevenK: "(ie. not a delayed copy or copy job)" [02:44] # If the changesfile is not None, it is an original upload [02:44] # (IE. not a delayed copy, or package copy job.) [02:44] Sounds reasonable. [02:50] wgrant: Diff updated. [02:53] Looking [02:57] StevenK: You'll need indices [02:58] :-( [02:58] Otherwise it seems to just go for a seq scan every time. [03:01] "CREATE INDEX temp_spph ON sourcepackagepublishinghistory USING btree (id) WHERE packageupload IS NULL;" [03:01] is probably all that's going to give us a wonderful result. [03:05] wgrant: i found the problem - there was some previous code which may have been cut and pasted and which did [self] instead of [self.bug] and so the wrong id was being used in the searches [03:06] wallyworld_: Ouch, where? [03:06] Using a task instead of a bug? [03:07] it was my code - i just can't quite recall which of the pipes in went into first - it is not landed yet, and the test it was in was failing due to waiting for the apa stuff to go in [03:07] Ah, right. [03:07] I didn't think I did anything like that anywhere. [03:08] so oen failure was masking another problem [03:09] although i have found an issue - the createAccessGrants method in the sharing service will fail if the grants already exist [03:09] so i need to fix that or else it will blow up when f flag is turned off but the triggers are still enabled [03:15] wallyworld_: btw, did you see lp.testing.dbtriggers? I guess you probably won't want to merge them now, so I'll sort it out once your stuff is landed. [03:16] wgrant: no, didn't see that yet [03:18] wgrant: you obviously didn't see my DisableTriggerFixture [03:19] wallyworld_: Not until afterwards -- it's not in trunk yet. [03:19] oh really!? :-( [03:19] didn't realise that [03:19] but makes sense [03:49] wallyworld_: Bug #1007984 may be of interest [03:49] <_mup_> Bug #1007984: BugTask:+index information type widget doesn't handle failure < https://launchpad.net/bugs/1007984 > [03:49] me or StevenK? [03:49] Both [03:50] but it's more fun to make him to javascript stuff :-) [03:50] s/to/do [03:50] True. [03:50] i know how much he loves it [03:53] JS was invented by Lucifer himself [03:55] i don't mind it now [03:56] You've clearly been infected. [03:58] wallyworld_: I figured out that it's a really good idea to re-flush the Breville after the clean cycle. Bleargh.... [03:58] bigjools: that's what she said in -ops this morning [04:15] wgrant: when do you plan on doing updateAccessPolicyArtifacts for branches? [04:16] wgrant: Did you see https://code.launchpad.net/~stevenk/launchpad/spph-packageupload-index/+merge/108515 ? [04:17] wgrant: I'm going to hold off on landing set-spph-packageupload until the index is on prod, thanks for the +1 [04:17] wallyworld_: Refactoring mostly done. [04:17] Shuffling around tests to make them less heinous now [04:18] wallyworld_: I don't understand enough about ChoiceSource, I guess, which is why that bug exists. [04:18] wgrant: ok, afaict as soon as that gets in all the tests in my ueber branch should pass hipefully [04:18] wallyworld_: Marvellous. [04:18] StevenK: i'll do it, i just wanted to get a reaction from you [04:19] wallyworld_: I'd be interested to see the diff [04:19] np, will do [04:21] wallyworld_: Besides, it's über ... [04:22] StevenK: ue or ü are aceptable [04:22] Only if you're on Windows. [04:22] Everywhere else has easy ways to enter ü, so there's no excuse. [04:23] I had to set my compose key :-( [04:23] sigh, i can type ue fatser than ü [04:23] same with ae, oe [04:23] you would have had a point if ue was somehow wrong [04:30] wgrant: when you have a minute, a small one, hopefully last branch in the ueber branch pipeline https://code.launchpad.net/~wallyworld/launchpad/createAccessGrants-robust/+merge/108518 [04:30] RAlt+" u isn't that much slower :) [04:31] extra key presses [04:32] wallyworld_: approved [04:33] wallyworld_: What about ç? [04:33] smartarse :-P [04:33] wgrant: thanks [04:34] wallyworld_: Or ö? [04:34] wallyworld_: What's the diffstat for the end of the pipe up to? [04:36] wgrant: i forget the command to get a merge preview diff [04:36] StevenK: oe === ö [04:37] Now now [04:37] == maybe [04:37] Not === [04:37] sticky fingers [04:38] JS disease [04:38] wallyworld_: ä? :-) [04:38] ae [04:38] what he said [04:39] Not to be confused with æ [04:39] Haha [04:39] I've forgotten how to type that one [04:39] æ [04:39] Compose+a e [04:39] Not too hard :) [04:39] Hm, so that works. :-) [04:48] wgrant: I wonder if spph.source_package_name can be murdered yet. [04:49] StevenK: API [04:49] What? It's exported? [04:49] It's the only way to get the name of the source package through the API. [04:49] That's the reason it was created. [04:50] Can't we just point it at self.sourcepackagename? [04:50] That's no string [04:50] That's an object [04:50] An unexported object! [04:50] you want to export SPN? die fiend [04:50] self.sourcepackagename.name then [04:51] You'll find that looks remarkably like source_package_name [04:51] ie. that should be identical to what it is today. [04:51] wgrant: It's still using self.sourcepackagerelease [04:51] Ah, right. [04:52] You can fix that if you really want. [04:52] It's that, or beating my head against Django [05:05] wgrant: https://code.launchpad.net/~stevenk/launchpad/denorm-spph-source_package_name/+merge/108520 [05:06] StevenK: Did you identify and fix the preloading callsites? [05:06] There are preloading callsites? :-( [05:06] There should be. [05:07] Anything that exposes that over the API is going to want to preload the SPN [05:07] It's possible that it doesn't; if you can't find any, file a bug. [05:09] wgrant: greping for \.source_package_name is mostly turning up DSDs, which are a whole other barrel of laughs, and tests. [05:09] StevenK: I'd hope so [05:10] Since they won't be touching that directly. [05:11] The current code wouldn't have pre-loaded SPR? [05:11] wut? [05:12] wgrant: SPPH.source_package_name is exported and grabs it via SPR. [05:12] StevenK: Yes. [05:12] So code will be preloading SPR and SPR.spn [05:15] wgrant: Right, it's SPR.sourcepackagename, so maybe a grep for \.sourcepackagename will be better [05:23] wgrant: I can not see anything related besides the dominator that uses load_related() over SPR [05:23] :( [05:23] Ignore, I guess [05:24] BMP and bugtask [05:24] But I'm not sure how that fits in [05:24] It doesn't. [05:30] wgrant: Can haz +1, or you no longer care and I should self-review? [05:30] StevenK: I'm not sure that it's an immensely valuable change. [05:31] But if you want to do it then it's probably a self-review. [05:31] wgrant: You don't think dropping SPR out of the chain is valuable? [05:32] We don't have a really good set of policies around the use of denormalised data. [05:32] I thought the policy was effectively "denormalised data is good, we should use it" [05:39] win 3 [06:42] * StevenK stabs Django [06:43] Error: Can't find the file 'settings.py' in the directory containing '/home/steven/auditorfixture/eggs/auditor-0.0.1-py2.7.egg/auditor/manage.pyc'. It appears you've customized things. [06:43] -rw-rw-r-- 1 steven steven 4.0K Jun 4 16:39 /home/steven/auditorfixture/eggs/auditor-0.0.1-py2.7.egg/auditor/settings.py === almaisan-away is now known as al-maisan [09:28] good morning === al-maisan is now known as almaisan-away [10:38] lifeless: ping === almaisan-away is now known as al-maisan === benji changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: benji | Firefighting: - | Critical bugs: 3.47*10^2 [13:04] Morning, all. === al-maisan is now known as almaisan-away === nhandler_ is now known as nhandler === salgado-lunch is now known as salgado === almaisan-away is now known as al-maisan === al-maisan is now known as almaisan-away [21:39] Could I have "Status: Approved" on https://code.launchpad.net/~cjwatson/launchpad/db-packageset-score-rename/+merge/108336 so that I can land it? [21:46] lifeless, should cjwatson be added to https://launchpad.net/~canonical-launchpad-committers ? [21:47] cjwatson, toggled, assuming it was just a formality [21:49] So I understand - thanks [22:20] jcsackett, StevenK, mumble [22:32] wallyworld_: http://pastebin.ubuntu.com/882822/ === benji changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: - | Firefighting: - | Critical bugs: 3.47*10^2 [23:16] james_w: I'm not here today; sick, but he probably meets the criteria I set out in my mail to canonical-launchpad