[00:36] <blr> hmm ended up ditching colander, validation isn't really complex enough to justify another dep, but worth looking at for larger services.
[04:47] <blr> wgrant: token invalidation all sorted, was thinking we'd run the sweeper every minute?
[04:57] <blr> would mean a token can potentially live 1m longer than it's ttl, although I would imagine most will be manually invalidated
[05:48] <wgrant> blr: 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:51] <blr> wgrant: it does, but what about the case where a token is manually invalidated and still within the ttl?
[05:51] <blr> I suppose I should check both
[05:53] <wgrant> Right, the pruning cronjob is just to clean up the DB.
[05:53] <blr> oh, I am checking both :P
[05:55] <blr> will add the cron job and schema migration to the charm tomorrow, then should be a in a reasonable state.
[05:56] <blr> I seem to be leaking 'a's... time for coffee.
[05:56] <wgrant> Heh
[05:56] <wgrant> Sounds good
[09:24] <cjwatson> wgrant: cross-ownership deletion constraints: we have those on recipes too, right?
[09:24] <wgrant> cjwatson: yes, and they're horrible.
[09:24] <wgrant> These are marginally worse (as they involve archives too), but not substantially.
[09:25] <cjwatson> I'd never noticed them before.  I assume it means that you can prevent somebody else from deleting their branch by creating a recipe for it
[09:25] <wgrant> Right, exactly.
[09:25] <wgrant> There are a couple of bugs about it.
[09:25] <cjwatson> How 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:26] <wgrant> Right, that might be reasonable.
[09:26] <wgrant> It is useful to compare the FKs to simple URLs from the snapcraft.yaml
[09:27] <wgrant> Only the root branch actually has the FK enforced.
[09:27] <wgrant> Because the rest are in freeform text from Launchpad's PoV.
[09:27] <cjwatson> Freeform but parsed, but yes.
[09:27] <wgrant> Not from Launchpad's PoV.
[09:28] <cjwatson> SourcePackageRecipeDataInstruction
[09:28] <cjwatson> Unless you're talking about snaps.
[09:29] <wgrant> Oh, yeah, I mean snaps.
[09:29] <wgrant> We can't enforce that references in snapcraft.yaml remain good.
[09:29] <wgrant> So perhaps we need not enforce that the reference *to* snapcraft.yaml remains good.
[09:29] <cjwatson> Right, hence why staleness doesn't work.
[09:30] <wgrant> One could argue that the branch/repo and the archive are just references that may become stale like any other.
[09:30] <wgrant> In fact the git_path has to be.
[09:30] <cjwatson> Indeed.
[09:31] <cjwatson> I'll weaken the constraint.
[09:32] <wgrant> It is similar to what we do with SPRB.recipe.
[09:33] <cjwatson> Weird place to break the link.
[09:34] <cjwatson> I definitely prefer having SnapBuild.snap non-nullable.
[09:34] <wgrant> For Snaps it is simpler, as the SnapBuilds are likely themselves deletable.
[09:34] <wgrant> It was not so with SourcePackageRecipeBuilds.
[09:34] <wgrant> Well, it would have been possible, but important authorship information would also have been lost.
[09:35] <wgrant> For packages in archives.
[09:35] <cjwatson> True
[09:35] <cjwatson> Also one of these days I shall remember that unique constraints are indexes.
[09:35] <wgrant> Indeed they are.
[09:37] <wgrant> cjwatson: Have you thought about how private branches/repos and snaps might work?
[09:42] <cjwatson> Not yet.  And hm, I'm missing information_type on snap, aren't I.
[09:42] <cjwatson> I think that's the only piece that requires consideration at the DB level
[09:42] <wgrant> It is conceivable.
[09:42] <cjwatson> Other than that it's mostly granting some kind of token to the builder
[09:43] <cjwatson> And I do need to handle processors as well; not just for restrictedness, but also just to configure which architectures to build for.
[09:44] <wgrant> That is true, if they're not going to be explicitly requested.
[09:44] <cjwatson> I 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:45] <cjwatson> Would be nicer to record which arches are currently being built and then have a button to build everything now pls.
[10:49] <cjwatson> wgrant: Perhaps SnapArch would be a reasonable thing to have?  I'm thinking it could pretty much exactly duplicate Archive.processors etc.
[10:49] <wgrant> cjwatson: That's certainly what I'd do.
[10:50] <cjwatson> Maybe without the enabled_restricted_processors stuff.
[10:51] <wgrant> Right, it doesn't particularly need the backward compat bits.
[10:51] <cjwatson> I mean, we want to avoid letting people set restricted processors, but it doesn't need that name.
[10:51] <wgrant> Oh, right.
[10:51] <wgrant> It may be a good opportunity to fix it for archives too.
[10:53] <cjwatson> I don't want to bite off any more just right now, my mouth is full already :P
[10:54] <wgrant> Damn.
[10:58] <cjwatson> Is there any point in having SnapArch.id, given that it'd be UNIQUE (snap, processor) anyway?
[11:01] <wgrant> No.
[11:01] <wgrant> ArchiveArch probably dates from not long after the Storm transition.
[11:11] <cjwatson> wgrant: OK.  Do my changes to https://code.launchpad.net/~cjwatson/launchpad/db-snappy/+merge/265332 look OK?
[11:12] <cjwatson> Updating the model code now
[11:14] <wgrant> 5+public.snaparch                         = SELECT, UPDATE
[11:14] <wgrant> orlyu
[11:15] <cjwatson> Hm yeah that's probably wrong
[11:15] <cjwatson> Nothing other than launchpad_main should need UPDATE
[11:16] <wgrant> Nothing should ever be updating them, should it?
[11:17] <cjwatson> Fair point.  Both fixed.
[11:17] <wgrant> I think that's reasonable then. Thanks.
[11:19] <cjwatson> I'll add and test Snap.processors before landing.
[11:28] <cjwatson> Oh, information_type, I forgot that too.
[12:27] <cjwatson> wgrant: 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 going
[12:35] <wgrant> cjwatson: 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:36] <wgrant> I see no benefit here; we'll be restricting it to two values anyway...
[12:37] <cjwatson> I guess if it's just private I can probably get that into the first pass.
[12:37] <wgrant> The main difficulty with privacy is working out who can see it.
[12:37] <wgrant> Since these things aren't within another privacy context (ignoring the team).
[12:38] <cjwatson> Right, that's why I was sort of inclined to defer to start with
[12:38] <wgrant> I cannot disagree.
[12:38] <wgrant> It is not significantly more difficult to retrofit.
[12:38] <cjwatson> And it's a bit multifaceted - do we hide the snap if the branch is invisible?
[12:39] <wgrant> Oh it is terrible, yes.
[12:39] <cjwatson> Though I suppose that just goes on the security adapter
[12:39] <cjwatson> But still
[12:39] <wgrant> We have the snap, the source, and the archive.
[12:39] <wgrant> All of which may be differently private.
[12:39] <wgrant> It is quite ghastly.
[12:41] <cjwatson> Anyway, I think I've addressed all your review comments now, so landing db-snappy
[12:42] <cjwatson> Let's see if I remembered everything this time
[12:50] <cjwatson> Oh, damnit, missing ON DELETE CASCADE
[12:51] <wgrant> cjwatson: Where were you going to use that?
[12:52] <cjwatson> SnapArch.snap
[12:52] <wgrant> Ah, I suppose that makes sense.
[12:55] <cjwatson> How 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] <wgrant> FKs aren't constraints.
[12:55] <cjwatson> Seems annoying not to be able to alter that in one step.
[12:56] <cjwatson> They kind of are.  \d lists them as such.
[12:56] <cjwatson> Also no ALTER TABLE ... DROP FOREIGN KEY
[12:56] <wgrant> Oh, that's right. They appear as constraints to some DML.
[12:57] <wgrant> I'm not sure I'd bother, anyway.
[12:57] <wgrant> There are other uncascadable FKs to Snap, so it needs a whole lot of custom deletion code anyawy.
[12:59] <cjwatson> I guess.  Just noticed when I merged up as far as snap-builds and tests started failing.
[13:00] <cjwatson> Its custom deletion code currently only consists of checking that there are no builds and then store.remove
[13:01] <cjwatson> With that check there are no other uncascadable FKs.
[13:02] <wgrant> No uncascadable FKs except the one that will affect anything that's actually been used, indeed :)
[13:03] <cjwatson> I 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:04] <wgrant> Right, exactly.
[13:15] <cjwatson> Snap.requestBuild is going to need some fixing up to honour processors, and maybe we want Snap.requestBuilds
[20:39] <blr> morning