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