| blr | hmm ended up ditching colander, validation isn't really complex enough to justify another dep, but worth looking at for larger services. | 00:36 |
|---|---|---|
| blr | wgrant: token invalidation all sorted, was thinking we'd run the sweeper every minute? | 04:47 |
| blr | would mean a token can potentially live 1m longer than it's ttl, although I would imagine most will be manually invalidated | 04:57 |
| 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:48 |
| 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:51 |
| wgrant | Right, the pruning cronjob is just to clean up the DB. | 05:53 |
| blr | oh, I am checking both :P | 05:53 |
| blr | will add the cron job and schema migration to the charm tomorrow, then should be a in a reasonable state. | 05:55 |
| blr | I seem to be leaking 'a's... time for coffee. | 05:56 |
| wgrant | Heh | 05:56 |
| wgrant | Sounds good | 05:56 |
| 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:24 |
| 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:25 |
| wgrant | Right, that might be reasonable. | 09:26 |
| wgrant | It is useful to compare the FKs to simple URLs from the snapcraft.yaml | 09:26 |
| 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:27 |
| cjwatson | SourcePackageRecipeDataInstruction | 09:28 |
| cjwatson | Unless you're talking about snaps. | 09:28 |
| 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:29 |
| 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:30 |
| cjwatson | I'll weaken the constraint. | 09:31 |
| wgrant | It is similar to what we do with SPRB.recipe. | 09:32 |
| cjwatson | Weird place to break the link. | 09:33 |
| 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:34 |
| 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:35 |
| wgrant | cjwatson: Have you thought about how private branches/repos and snaps might work? | 09:37 |
| 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:42 |
| 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:43 |
| 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:44 |
| cjwatson | Would be nicer to record which arches are currently being built and then have a button to build everything now pls. | 09:45 |
| 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:49 |
| cjwatson | Maybe without the enabled_restricted_processors stuff. | 10:50 |
| 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:51 |
| cjwatson | I don't want to bite off any more just right now, my mouth is full already :P | 10:53 |
| wgrant | Damn. | 10:54 |
| cjwatson | Is there any point in having SnapArch.id, given that it'd be UNIQUE (snap, processor) anyway? | 10:58 |
| wgrant | No. | 11:01 |
| wgrant | ArchiveArch probably dates from not long after the Storm transition. | 11:01 |
| cjwatson | wgrant: OK. Do my changes to https://code.launchpad.net/~cjwatson/launchpad/db-snappy/+merge/265332 look OK? | 11:11 |
| cjwatson | Updating the model code now | 11:12 |
| wgrant | 5+public.snaparch = SELECT, UPDATE | 11:14 |
| wgrant | orlyu | 11:14 |
| cjwatson | Hm yeah that's probably wrong | 11:15 |
| cjwatson | Nothing other than launchpad_main should need UPDATE | 11:15 |
| wgrant | Nothing should ever be updating them, should it? | 11:16 |
| cjwatson | Fair point. Both fixed. | 11:17 |
| wgrant | I think that's reasonable then. Thanks. | 11:17 |
| cjwatson | I'll add and test Snap.processors before landing. | 11:19 |
| cjwatson | Oh, information_type, I forgot that too. | 11:28 |
| 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:27 |
| 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:35 |
| wgrant | I see no benefit here; we'll be restricting it to two values anyway... | 12:36 |
| 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:37 |
| 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:38 |
| 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:39 |
| cjwatson | Anyway, I think I've addressed all your review comments now, so landing db-snappy | 12:41 |
| cjwatson | Let's see if I remembered everything this time | 12:42 |
| cjwatson | Oh, damnit, missing ON DELETE CASCADE | 12:50 |
| wgrant | cjwatson: Where were you going to use that? | 12:51 |
| cjwatson | SnapArch.snap | 12:52 |
| wgrant | Ah, I suppose that makes sense. | 12:52 |
| 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:55 |
| 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:56 |
| 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:57 |
| cjwatson | I guess. Just noticed when I merged up as far as snap-builds and tests started failing. | 12:59 |
| cjwatson | Its custom deletion code currently only consists of checking that there are no builds and then store.remove | 13:00 |
| cjwatson | With that check there are no other uncascadable FKs. | 13:01 |
| wgrant | No uncascadable FKs except the one that will affect anything that's actually been used, indeed :) | 13:02 |
| 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:03 |
| wgrant | Right, exactly. | 13:04 |
| cjwatson | Snap.requestBuild is going to need some fixing up to honour processors, and maybe we want Snap.requestBuilds | 13:15 |
| blr | morning | 20:39 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!