| 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:14 |
|---|---|---|
| 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:15 |
| 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:16 |
| 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:17 |
| 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:18 |
| 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:19 |
| 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:20 |
| 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:21 |
| StevenK | wgrant, wallyworld_: https://code.launchpad.net/~stevenk/launchpad/set-spph-packageupload/+merge/108513 | 01:26 |
| 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:28 |
| 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:29 |
| 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:30 |
| StevenK | Hmm, I'm not sure how to check it's the earliest SPPH | 01:31 |
| 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:32 |
| StevenK | wgrant: Hmm, make sense. Now there's this Storm thing. Let me see. | 01:36 |
| wgrant | Easy in Storm. | 01:36 |
| StevenK | wgrant: Hm, really? My 5 minutes of playing around has yielded nothing. | 01:40 |
| 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:41 |
| StevenK | Oh Select() | 01:42 |
| wgrant | That is how one does a subselect. | 01:42 |
| StevenK | I was trying to do it directly :-( | 01:42 |
| 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:43 |
| wgrant | Ah, forgot to match section | 01:45 |
| wgrant | 31, that's more plausible | 01:45 |
| 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:49 |
| 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:50 |
| StevenK | wgrant: Yeah. Writing a test for it | 01:52 |
| 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:53 |
| 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:57 |
| 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! ;) | 01:58 |
| 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:01 |
| StevenK | wgrant: http://pastebin.ubuntu.com/1022402/ | 02:31 |
| 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:32 |
| 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:33 |
| 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:34 |
| 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:35 |
| 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:36 |
| wallyworld_ | ok, will do after i look into things a bit more | 02:38 |
| StevenK | wgrant: http://pastebin.ubuntu.com/1022422/ | 02:42 |
| wgrant | StevenK: "(ie. not a delayed copy or copy job)" | 02:43 |
| 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:44 |
| StevenK | wgrant: Diff updated. | 02:50 |
| wgrant | Looking | 02:53 |
| wgrant | StevenK: You'll need indices | 02:57 |
| StevenK | :-( | 02:58 |
| wgrant | Otherwise it seems to just go for a seq scan every time. | 02:58 |
| 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:01 |
| 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:05 |
| wgrant | wallyworld_: Ouch, where? | 03:06 |
| wgrant | Using a task instead of a bug? | 03:06 |
| 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:07 |
| wallyworld_ | so oen failure was masking another problem | 03:08 |
| 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:09 |
| 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:15 |
| wallyworld_ | wgrant: no, didn't see that yet | 03:16 |
| wallyworld_ | wgrant: you obviously didn't see my DisableTriggerFixture | 03:18 |
| 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:19 |
| 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:49 |
| 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:50 |
| bigjools | JS was invented by Lucifer himself | 03:53 |
| wallyworld_ | i don't mind it now | 03:55 |
| wgrant | You've clearly been infected. | 03:56 |
| 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 | 03:58 |
| wallyworld_ | wgrant: when do you plan on doing updateAccessPolicyArtifacts for branches? | 04:15 |
| StevenK | wgrant: Did you see https://code.launchpad.net/~stevenk/launchpad/spph-packageupload-index/+merge/108515 ? | 04:16 |
| 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:17 |
| 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:18 |
| StevenK | wallyworld_: I'd be interested to see the diff | 04:19 |
| wallyworld_ | np, will do | 04:19 |
| StevenK | wallyworld_: Besides, it's über ... | 04:21 |
| 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:22 |
| 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:23 |
| 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:30 |
| wallyworld_ | extra key presses | 04:31 |
| wgrant | wallyworld_: approved | 04:32 |
| StevenK | wallyworld_: What about ç? | 04:33 |
| wallyworld_ | smartarse :-P | 04:33 |
| wallyworld_ | wgrant: thanks | 04:33 |
| StevenK | wallyworld_: Or ö? | 04:34 |
| wgrant | wallyworld_: What's the diffstat for the end of the pipe up to? | 04:34 |
| wallyworld_ | wgrant: i forget the command to get a merge preview diff | 04:36 |
| wallyworld_ | StevenK: oe === ö | 04:36 |
| wgrant | Now now | 04:37 |
| wgrant | == maybe | 04:37 |
| wgrant | Not === | 04:37 |
| wallyworld_ | sticky fingers | 04:37 |
| StevenK | JS disease | 04:38 |
| StevenK | wallyworld_: ä? :-) | 04:38 |
| wgrant | ae | 04:38 |
| wallyworld_ | what he said | 04:38 |
| 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:39 |
| StevenK | wgrant: I wonder if spph.source_package_name can be murdered yet. | 04:48 |
| 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:49 |
| 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:50 |
| 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:51 |
| wgrant | You can fix that if you really want. | 04:52 |
| StevenK | It's that, or beating my head against Django | 04:52 |
| StevenK | wgrant: https://code.launchpad.net/~stevenk/launchpad/denorm-spph-source_package_name/+merge/108520 | 05:05 |
| wgrant | StevenK: Did you identify and fix the preloading callsites? | 05:06 |
| StevenK | There are preloading callsites? :-( | 05:06 |
| wgrant | There should be. | 05:06 |
| 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:07 |
| 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:09 |
| wgrant | Since they won't be touching that directly. | 05:10 |
| StevenK | The current code wouldn't have pre-loaded SPR? | 05:11 |
| wgrant | wut? | 05:11 |
| 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:12 |
| StevenK | wgrant: Right, it's SPR.sourcepackagename, so maybe a grep for \.sourcepackagename will be better | 05:15 |
| 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:23 |
| StevenK | BMP and bugtask | 05:24 |
| StevenK | But I'm not sure how that fits in | 05:24 |
| wgrant | It doesn't. | 05:24 |
| 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:30 |
| 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:31 |
| 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:32 |
| wgrant | win 3 | 05:39 |
| * StevenK stabs Django | 06:42 | |
| 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 | 06:43 |
| === almaisan-away is now known as al-maisan | ||
| adeuring | good morning | 09:28 |
| === al-maisan is now known as almaisan-away | ||
| frankban | lifeless: ping | 10:38 |
| === 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 | ||
| deryck | Morning, all. | 13:04 |
| === 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 | ||
| 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:39 |
| james_w | lifeless, should cjwatson be added to https://launchpad.net/~canonical-launchpad-committers ? | 21:46 |
| james_w | cjwatson, toggled, assuming it was just a formality | 21:47 |
| cjwatson | So I understand - thanks | 21:49 |
| sinzui | jcsackett, StevenK, mumble | 22:20 |
| wgrant | wallyworld_: http://pastebin.ubuntu.com/882822/ | 22:32 |
| === benji changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: - | Firefighting: - | Critical bugs: 3.47*10^2 | ||
| lifeless | james_w: I'm not here today; sick, but he probably meets the criteria I set out in my mail to canonical-launchpad | 23:16 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!