[00:00] <wallyworld> but the whole method can be refactored since it duplicates the override stuff
[00:00] <wallyworld> not just for unknown files
[00:01] <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
[04:39] <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:40] <StevenK> wgrant: Yes it does? We need to instruct the remote site which one we're talking to.
[04:41] <wgrant> +        return {'User-agent': LP_USER_AGENT, 'Host': self.baseurl}
[04:41] <wgrant> I spy with my little eye a contradictory statement
[04:42] <wgrant> (host != URL)
[04:42] <StevenK> Bleh
[04:46] <StevenK> wgrant: Changes pushing, thanks.
[04:49] <StevenK> Diff updated.
[04:50] <wgrant> Thanks
[05:39] <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:40] <wgrant> wallyworld: Sure
[05:41] <wallyworld> thanks, no hurry
[05:41] <StevenK> Blink
[05:41] <StevenK> db-devel died
[05:41] <wallyworld> what?
[05:45] <StevenK> subunit corruption, looks like
[05:48] <wallyworld> \o/
[05:56] <wallyworld> wgrant: can tables be created live with no locks? i can't see why not
[05:57] <wgrant> wallyworld: Only if they have no foreign keys
[05:58] <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:59] <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
[06:00] <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:01] <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:02] <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:03] <wallyworld> the business logic changes to consume it are trivial, and actually reduce LOC I think
[09:34] <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:35] <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:36] <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:37] <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:38] <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:39] <stub> oh, haven't finished looking at it yet :)
[09:39] <wallyworld> ok
[09:43] <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:44] <stub> wallyworld: no
[09:44] <wallyworld> bah, archive_purpose
[09:44] <wallyworld> just about to push changes
[09:44] <wallyworld> thanks for looking
[09:48] <stub> And now I read your statement about FKs in the MP :-/
[09:51] <wallyworld> stub: changes pushed
[09:51] <wgrant> stub, wallyworld: That table definitely needs foreign keys
[09:52] <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:53] <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:54] <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:55] <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:56] <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:57] <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:58] <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:59] <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.
[10:00] <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:01] <wallyworld> np, thanks
[10:02] <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:03] <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:04] <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:05]  * 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:06] <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:07] <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:08] <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:09] <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:10] <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:11] <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:12] <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:13] <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:14] <wgrant> They're Registry
[10:14] <wgrant> :)
[10:14] <wallyworld> well, clearly i have a lot to learn in this domain area
[10:18] <wallyworld> stub: ignoring the tablename etc, is this correct for creating the surrogate id? https://pastebin.canonical.com/77517/
[10:20] <wgrant> "id serial PRIMARY KEY"
[10:20] <wgrant> is all you need
[10:21] <wgrant> It will autocreate the sequence etc
[10:22] <wgrant> wallyworld: ^^
[10:22] <wallyworld> ok
[10:23] <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:24] <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
[15:55] <sinzui> jcsackett, do you have time to review https://code.launchpad.net/~sinzui/launchpad/reimport-inactive-templates/+merge/132421
[15:55] <jcsackett> sinzui: sure.
[16:08] <jcsackett> sinzui: looks good, just want to make sure that the first check returning `entry` if true and others returnin None is intentional?
[16:09] <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:10] <jcsackett> sinzui: r=me.
[16:10] <sinzui> thank you
[16:52] <czajkowski> cjwatson: people think you sleep! little do they know!
[17:37] <sinzui> abentley, I think your PRF bug and my person merge bug can be fixed in the same branch
[18:49] <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:50] <sinzui> I will resolve conflicts with your branch
[18:50] <abentley> sinzui: thanks.
[19:39] <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:54] <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.
[20:07] <sinzui> jcsackett, Do you have time to review a branch https://code.launchpad.net/~sinzui/launchpad/merge-db-permissions/+merge/132615
[20:08] <jcsackett> sinzui: looking now.
[20:10] <jcsackett> sinzui: r=me.
[20:11] <sinzui> thank you
[20:48] <abentley> jcsackett: Could you please review https://code.launchpad.net/~abentley/launchpad/fix-assignment-list/+merge/132621 ?
[20:49] <jcsackett> abentley: sure.
[20:50] <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.