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