[00:00] but the whole method can be refactored since it duplicates the override stuff [00:00] not just for unknown files [00:01] the policy can be got : override_policy = self.policy.archive.getOverridePolicy() [00:01] and then the adaptor can be used [00:01] wallyworld: Approved [00:01] thans === Ursinha is now known as Ursinha-afk [04:39] wallyworld, wgrant: https://code.launchpad.net/~stevenk/launchpad/send-host-header-during-getremotebug/+merge/132447 [04:39] StevenK: The Host header doesn't work like that [04:40] wgrant: Yes it does? We need to instruct the remote site which one we're talking to. [04:41] + return {'User-agent': LP_USER_AGENT, 'Host': self.baseurl} [04:41] I spy with my little eye a contradictory statement [04:42] (host != URL) [04:42] Bleh [04:46] wgrant: Changes pushing, thanks. [04:49] Diff updated. [04:50] Thanks [05:39] wgrant: since you ran the queries on DF, if you get a moment at some point, could you look at this for me? https://code.launchpad.net/~wallyworld/launchpad/person-index-timeout-931771/+merge/132446 [05:40] wallyworld: Sure [05:41] thanks, no hurry [05:41] Blink [05:41] db-devel died [05:41] what? [05:45] subunit corruption, looks like [05:48] \o/ [05:56] wgrant: can tables be created live with no locks? i can't see why not [05:57] wallyworld: Only if they have no foreign keys [05:58] it does, but the table will be empty [05:58] Adding a foreign key creates an internal trigger on both ends, which requires an access exclusive lock [05:58] surely only for a micro second [05:58] Right, as with most ALTER TABLEs it's basically instantaneous [05:59] but we still cannot create live? [05:59] Not without delaying things for maximum_txn_length * 2 [05:59] it would be no worse than may of our transactions [05:59] the ALTER TABLE will request an exclusive lock on the table [05:59] this will wait around until all existing transactions finish [06:00] the pending, ungranted lock will block any new transactions that want to use the table [06:00] Once all the existing transactions finish, the lock can be taken and then released [06:00] But we have lots of long transactions, so this could block everything for minutes or even longer [06:00] And given FDT is not really an issue [06:00] ok, FDT it is [06:01] What new table? [06:01] for recording the latest published sources [06:01] source packages [06:01] BTF for source packages? [06:01] BTF? [06:02] BugTaskFlat [06:02] well, not really [06:02] but it is a denorm of the data model for reporting [06:02] it will be maintained by a garbo frequently job initially [06:03] the business logic changes to consume it are trivial, and actually reduce LOC I think === deryck_ is now known as deryck === Ursinha-afk is now known as Ursinha [09:34] wallyworld: You are missing foreign key definitions in that MP. Is that deliberate or an oversight? [09:34] stub: deliberate [09:34] it's only a reporting table [09:34] so i didn't want to do anything to slow down performance [09:35] AFAIK, BugTaskFlat has no FKs also [09:35] bt i could be wrong [09:35] Yeah, but it is maintained entirely by triggers to enforce referential integrity [09:35] i'm happy to add them if they are required [09:36] do the triggers even check for missing references? [09:36] wallyworld: They are not required, but leaving them out is a potential foot gun [09:36] eg with BugTaskFlat, what happens if a referrenfed object is deleted? [09:36] i don't think that is caught [09:36] It should be [09:37] should be or is? [09:37] i'll add the FK references to the mp though [09:37] it should be, as that is what was supposed to happen when I reviewed it IIRC [09:37] o [09:37] Otherwise we would be giving out incorrect results (the footgun) [09:37] ok [09:38] So here for instance, missing FK to the person table could mean packages get lost when a person merge happens. [09:38] so i just need to update the table definition to say REFERENCES foo, nothing else? [09:38] Declaring them 'on delete cascade' may be useful, or may be a big problem [09:38] wallyworld: yes [09:39] oh, haven't finished looking at it yet :) [09:39] ok [09:43] wallyworld: Yeah, all looks fine apart from the FK issue [09:43] stub: what about the archive.purpose column? [09:43] which is an enum [09:43] do i need a fk there too? [09:43] wallyworld: If they are a problem, we can leave them out but we have to think hard about potential problems [09:44] wallyworld: no [09:44] bah, archive_purpose [09:44] just about to push changes [09:44] thanks for looking [09:48] And now I read your statement about FKs in the MP :-/ [09:51] stub: changes pushed [09:51] stub, wallyworld: That table definitely needs foreign keys [09:52] they are there now :-) [09:52] BugTaskFlat doesn't, since it's maintained entirely inline by triggers on FKed tables [09:52] so i will need to update the person merge code when i do the devel work [09:52] And there's a one-to-one relationship between BugTask and BugTaskFlat [09:52] wallyworld: Possibly [09:53] wallyworld: Person merge will crash (and tests will fail) if you add a new person FK that's involved in a unique constraint [09:53] oh, archive_purpose needs to remain in sync with Archive.purpose? [09:53] yes [09:53] Archive.purpose doesn't change [09:54] wallyworld: Should dateuploaded and publication be NOT NULL? [09:54] Also, that table name is far too generic [09:54] wgrant: publication is a reference to SPPH [09:54] wallyworld: Sure [09:54] It still needs to be NOT NULL [09:54] oh, i meant to put not null [09:54] It also can't be the PK [09:55] As it's mutable [09:55] i thought primary keys were automatically not null [09:55] Well, it could be the PK, but Storm will kill you in your sleep if you try to change it [09:55] ah, right. i was thinking the pib reference would not change [09:55] publication i mean [09:55] but it can of course [09:56] i'd add a surrogate key [09:56] The natural key is (person, archive, distroseries) [09:56] which person? [09:56] That's the question [09:56] creator? [09:57] I'm not quite sure that this schema does what we need; we'll probably want a complete garbo+browser implementation reviewed before this landss [09:57] i did the natural key as spn, archive, distroseries [09:57] Er, yeah, spn needs to be in there as well [09:57] But person obviously has to be too [09:57] not obvious to me :-) [09:57] Hm? [09:57] since i don;t know soyuz that well [09:57] Why not? [09:58] We care about the person's latest package [09:58] s [09:58] i thought only one person would publish a spn, archive, distroseries [09:58] http://paste.ubuntu.com/1323048/ enforces the purpose can't get changed on archive [09:58] wallyworld: Only one person's publication will be current, sure [09:58] But this shows superseded ones too [09:58] thanks stub, will add that [09:58] ah, ok [09:59] so creator i suppose is the person to use [09:59] Well [09:59] http://paste.ubuntu.com/1323050/ is probably better, or it will be a PITA to change the purpose of an archive [09:59] It probably needs to be both [09:59] stub: We never want to change the purpose of an archive. [10:00] It is by all accounts immutable [10:00] Right, archive purposes cannot change without manual SQL [10:00] I don't think one has ever changed [10:00] It doesn't make sense [10:00] stub: is the preference to add constraints inline or use an alter table? [10:00] If one wants to change, we have bigger problems [10:00] So probably no need for the extra index and multi column FK constraint [10:00] This is a cache table that is cheap to regenerate and unproblematic if it breaks [10:00] wallyworld: technically it makes no difference. I find it all inline more readable myself. [10:01] np, thanks [10:02] stub: with the comments, there's a comments.sql - does that get rolled up at some point? [10:02] comments.sql no longer gets applied to production, either [10:02] czajkowski, did the lp session get dropped today? [10:03] wallyworld: The comments should go in comments.sql sorry. [10:03] deryck: it did, there was no other sign ups and didnt want to waste the lP dev time [10:03] stub: ok, i saw some other patches contained them, so wasn;t sure [10:03] czajkowski, ah, ok. Thanks for putting those sessions together. [10:04] no worires thanks for coming ;) [10:04] yeah, and your question made me question that. [10:04] stub: i thought it might be like patches - at some point, a rollup is done if you know what i mean [10:04] so add everything to the patch, incl comments [10:04] yeah, but we haven't written any code to do that rollup [10:05] * stub thinks [10:05] ok, will just add to comments.sql then [10:05] wallyworld: nah [10:05] wallyworld: leave them where they are. The rollup will happen [10:05] ok [10:06] I should apply comments.sql to production and then nuke it so I don't confuse myself again [10:06] wgrant: i'll add both creator and maintainer to the natural key for now and we can rethink before landing [10:07] The schema will likely need to evolve once we discover that the code cannot use it [10:07] reading scrollback, the natural key is complex enough to warrent just using an 'id' serial column [10:08] yeah, adding [10:08] i never use natural key as pk [10:08] i only ever add a constraint to enforce it [10:09] i mistakenly thought i could use the publication reference as the pk, thinking there was a 1-1 relationship [10:09] This table will never be referenced in an FK, so a natural key makes a reasonable amount of sense, but it's not going to run off the end of a 32-bit int any time soon, so it doesn't matter much either way [10:09] But we should avoid surrogate keys when we can :) [10:09] i disagree :-) [10:10] Ah, so it was you who designed branchrevision :P [10:10] surrogate keys form the basis of good db design [10:10] And oauthnonce [10:10] not for join tables necessarily [10:10] but for straight up entity tables, yes [10:10] Surrogate keys for things that need to be referenced by FKs, sure [10:11] This isn't an entity table [10:11] As such [10:11] sure [10:11] what would you prefer the table name to be? [10:11] Something involving person, sourcepackagerelease, and cache, probably. [10:12] LatestPublishedReleases doesn't mean anything [10:12] What do we do when we want a table showing the latest releases for a product, or binarypackagereleases, or... [10:12] shouldn't the table name contain 'latest'? [10:12] Probably [10:12] i thought all releases went into spph [10:12] and spr [10:13] Source package releases go into SPR and SPPH [10:13] Binary package release don't [10:13] Product releases don't [10:13] i didn't know there were any other kinds [10:13] Because they're not source package releases :) [10:13] ok, i know that *now* :-) [10:13] remember, i am a soyuz virgin [10:13] Product releases aren't Soyuz [10:14] They're Registry [10:14] :) [10:14] well, clearly i have a lot to learn in this domain area [10:18] stub: ignoring the tablename etc, is this correct for creating the surrogate id? https://pastebin.canonical.com/77517/ [10:20] "id serial PRIMARY KEY" [10:20] is all you need [10:21] It will autocreate the sequence etc [10:22] wallyworld: ^^ [10:22] ok [10:23] was the stuff i used just old code? [10:23] That's probably how they appear in dumps [10:23] serial maps to that [10:23] ok [10:24] i looked at our sql files though [10:24] But serial has been around forever. Doing it manually is not so much "old" as "positively ancient" [10:24] The base schema? That's pg_dump output [10:24] Not manually written [10:24] not sure where i found it now, probably === Ursinha is now known as Ursinha-afk === Ursinha-afk is now known as Ursinha === jcsackett changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: jcsackett | Firefighting: - | Critical bugs: ~200 === Ursinha is now known as Ursinha-afk === Ursinha-afk is now known as Ursinha === matsubara is now known as matsubara-lunch [15:55] jcsackett, do you have time to review https://code.launchpad.net/~sinzui/launchpad/reimport-inactive-templates/+merge/132421 [15:55] sinzui: sure. [16:08] sinzui: looks good, just want to make sure that the first check returning `entry` if true and others returnin None is intentional? [16:09] yes, because we do not want to tamper with it [16:09] sinzui: ok, that's what i thought, but i figured i would double check. [16:10] sinzui: r=me. [16:10] thank you === beuno is now known as beuno-lunch === beuno-lunch is now known as beuno [16:52] cjwatson: people think you sleep! little do they know! === Ursinha is now known as Ursinha-afk === matsubara-lunch is now known as matsubara [17:37] abentley, I think your PRF bug and my person merge bug can be fixed in the same branch [18:49] sinzui: Mine's already landing via EC2. [18:49] excellent [18:49] I am moving SELECT to the script group so that no script needs basic access [18:50] I will resolve conflicts with your branch [18:50] sinzui: thanks. [19:39] sinzui: does the db buildbot failure prevent regular devel landings? pqm is yelling about my commit msg not satisfying the testfix requirement but it doesn't look like we should need one... [19:54] jcsackett, one failure breaks both. You need to land a fix correct builder or force a build for spurious [19:54] sinzui: ah. for some reason i thought they were separate. [20:07] jcsackett, Do you have time to review a branch https://code.launchpad.net/~sinzui/launchpad/merge-db-permissions/+merge/132615 [20:08] sinzui: looking now. [20:10] sinzui: r=me. [20:11] thank you [20:48] jcsackett: Could you please review https://code.launchpad.net/~abentley/launchpad/fix-assignment-list/+merge/132621 ? [20:49] abentley: sure. [20:50] abentley: that was short. :-) r=me. [20:50] jcsackett: Thanks. [20:50] jcsackett: If only all critical bugs were so simple to fix :-) [20:50] abentley: i hear that. === jcsackett changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: - | Firefighting: - | Critical bugs: ~200 === almaisan-away is now known as al-maisan === al-maisan is now known as almaisan-away