blrhmm ended up ditching colander, validation isn't really complex enough to justify another dep, but worth looking at for larger services.00:36
blrwgrant: token invalidation all sorted, was thinking we'd run the sweeper every minute?04:47
blrwould mean a token can potentially live 1m longer than it's ttl, although I would imagine most will be manually invalidated04:57
wgrantblr: I don't think it's particularly necessary to run it that frequently. Presumably the thing validating a token would check the TTL before allowing it.05:48
blrwgrant: it does, but what about the case where a token is manually invalidated and still within the ttl?05:51
blrI suppose I should check both05:51
wgrantRight, the pruning cronjob is just to clean up the DB.05:53
blroh, I am checking both :P05:53
blrwill add the cron job and schema migration to the charm tomorrow, then should be a in a reasonable state.05:55
blrI seem to be leaking 'a's... time for coffee.05:56
wgrantSounds good05:56
cjwatsonwgrant: cross-ownership deletion constraints: we have those on recipes too, right?09:24
wgrantcjwatson: yes, and they're horrible.09:24
wgrantThese are marginally worse (as they involve archives too), but not substantially.09:24
cjwatsonI'd never noticed them before.  I assume it means that you can prevent somebody else from deleting their branch by creating a recipe for it09:25
wgrantRight, exactly.09:25
wgrantThere are a couple of bugs about it.09:25
cjwatsonHow about making them nullable in this case?  You wouldn't be able to issue any new builds, but the existing snap builds could stick around/.09:25
wgrantRight, that might be reasonable.09:26
wgrantIt is useful to compare the FKs to simple URLs from the snapcraft.yaml09:26
wgrantOnly the root branch actually has the FK enforced.09:27
wgrantBecause the rest are in freeform text from Launchpad's PoV.09:27
cjwatsonFreeform but parsed, but yes.09:27
wgrantNot from Launchpad's PoV.09:27
cjwatsonUnless you're talking about snaps.09:28
wgrantOh, yeah, I mean snaps.09:29
wgrantWe can't enforce that references in snapcraft.yaml remain good.09:29
wgrantSo perhaps we need not enforce that the reference *to* snapcraft.yaml remains good.09:29
cjwatsonRight, hence why staleness doesn't work.09:29
wgrantOne could argue that the branch/repo and the archive are just references that may become stale like any other.09:30
wgrantIn fact the git_path has to be.09:30
cjwatsonI'll weaken the constraint.09:31
wgrantIt is similar to what we do with SPRB.recipe.09:32
cjwatsonWeird place to break the link.09:33
cjwatsonI definitely prefer having SnapBuild.snap non-nullable.09:34
wgrantFor Snaps it is simpler, as the SnapBuilds are likely themselves deletable.09:34
wgrantIt was not so with SourcePackageRecipeBuilds.09:34
wgrantWell, it would have been possible, but important authorship information would also have been lost.09:34
wgrantFor packages in archives.09:35
cjwatsonAlso one of these days I shall remember that unique constraints are indexes.09:35
wgrantIndeed they are.09:35
wgrantcjwatson: Have you thought about how private branches/repos and snaps might work?09:37
cjwatsonNot yet.  And hm, I'm missing information_type on snap, aren't I.09:42
cjwatsonI think that's the only piece that requires consideration at the DB level09:42
wgrantIt is conceivable.09:42
cjwatsonOther than that it's mostly granting some kind of token to the builder09:42
cjwatsonAnd I do need to handle processors as well; not just for restrictedness, but also just to configure which architectures to build for.09:43
wgrantThat is true, if they're not going to be explicitly requested.09:44
cjwatsonI guess we don't have daily builds yet so it could be entirely a matter of explicit requests, but that seems like poor UX.09:44
cjwatsonWould be nicer to record which arches are currently being built and then have a button to build everything now pls.09:45
cjwatsonwgrant: Perhaps SnapArch would be a reasonable thing to have?  I'm thinking it could pretty much exactly duplicate Archive.processors etc.10:49
wgrantcjwatson: That's certainly what I'd do.10:49
cjwatsonMaybe without the enabled_restricted_processors stuff.10:50
wgrantRight, it doesn't particularly need the backward compat bits.10:51
cjwatsonI mean, we want to avoid letting people set restricted processors, but it doesn't need that name.10:51
wgrantOh, right.10:51
wgrantIt may be a good opportunity to fix it for archives too.10:51
cjwatsonI don't want to bite off any more just right now, my mouth is full already :P10:53
cjwatsonIs there any point in having SnapArch.id, given that it'd be UNIQUE (snap, processor) anyway?10:58
wgrantArchiveArch probably dates from not long after the Storm transition.11:01
cjwatsonwgrant: OK.  Do my changes to https://code.launchpad.net/~cjwatson/launchpad/db-snappy/+merge/265332 look OK?11:11
cjwatsonUpdating the model code now11:12
wgrant5+public.snaparch                         = SELECT, UPDATE11:14
cjwatsonHm yeah that's probably wrong11:15
cjwatsonNothing other than launchpad_main should need UPDATE11:15
wgrantNothing should ever be updating them, should it?11:16
cjwatsonFair point.  Both fixed.11:17
wgrantI think that's reasonable then. Thanks.11:17
cjwatsonI'll add and test Snap.processors before landing.11:19
cjwatsonOh, information_type, I forgot that too.11:28
cjwatsonwgrant: If you're still up, do you think I should have Snap.information_type (like proper shareable things) or just Snap.private (like Archive)?  Maybe I should defer privacy handling until a bit later, or I'll never get this going12:27
wgrantcjwatson: I would prefer Snap.private, but I have long had a potentially unreasonable vendetta against information_type, so take it with a grain of salt.12:35
wgrantI see no benefit here; we'll be restricting it to two values anyway...12:36
cjwatsonI guess if it's just private I can probably get that into the first pass.12:37
wgrantThe main difficulty with privacy is working out who can see it.12:37
wgrantSince these things aren't within another privacy context (ignoring the team).12:37
cjwatsonRight, that's why I was sort of inclined to defer to start with12:38
wgrantI cannot disagree.12:38
wgrantIt is not significantly more difficult to retrofit.12:38
cjwatsonAnd it's a bit multifaceted - do we hide the snap if the branch is invisible?12:38
wgrantOh it is terrible, yes.12:39
cjwatsonThough I suppose that just goes on the security adapter12:39
cjwatsonBut still12:39
wgrantWe have the snap, the source, and the archive.12:39
wgrantAll of which may be differently private.12:39
wgrantIt is quite ghastly.12:39
cjwatsonAnyway, I think I've addressed all your review comments now, so landing db-snappy12:41
cjwatsonLet's see if I remembered everything this time12:42
cjwatsonOh, damnit, missing ON DELETE CASCADE12:50
wgrantcjwatson: Where were you going to use that?12:51
wgrantAh, I suppose that makes sense.12:52
cjwatsonHow do I add that after the fact?  ALTER TABLE SnapArch DROP CONSTRAINT snaparch_arch_fkey; ALTER TABLE SnapArch ADD FOREIGN KEY (snap) REFERENCES snap ON DELETE CASCADE?12:55
wgrantFKs aren't constraints.12:55
cjwatsonSeems annoying not to be able to alter that in one step.12:55
cjwatsonThey kind of are.  \d lists them as such.12:56
cjwatsonAlso no ALTER TABLE ... DROP FOREIGN KEY12:56
wgrantOh, that's right. They appear as constraints to some DML.12:56
wgrantI'm not sure I'd bother, anyway.12:57
wgrantThere are other uncascadable FKs to Snap, so it needs a whole lot of custom deletion code anyawy.12:57
cjwatsonI guess.  Just noticed when I merged up as far as snap-builds and tests started failing.12:59
cjwatsonIts custom deletion code currently only consists of checking that there are no builds and then store.remove13:00
cjwatsonWith that check there are no other uncascadable FKs.13:01
wgrantNo uncascadable FKs except the one that will affect anything that's actually been used, indeed :)13:02
cjwatsonI suppose in future we might want to be able to delete those.  It makes a bit more sense than it would for recipes where they'd have to cascade into package builds and obviously can't.13:03
wgrantRight, exactly.13:04
cjwatsonSnap.requestBuild is going to need some fixing up to honour processors, and maybe we want Snap.requestBuilds13:15

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!