[13:13] <cjwatson> Hm, turnip authentication is suboptimally-designed for code imports: it authenticates before it knows the repository, and the only state it retains between authentication and path translation is Person.id.
[13:14] <cjwatson> I might have to change it to pass the username again if the user ID is None, and make up a username that's something like +code-import-job/<job_id> (to fit the blacklist)
[13:22] <wgrant> cjwatson: Hm, do we really pass the ID as an int?
[13:23] <wgrant> I think it's reasonable for authentication to be contextless; after all, in the SSH case we don't have context until authentication completes.
[13:29] <cjwatson> We really pass the ID as an int.
[13:29] <wgrant> :(
[13:30] <cjwatson> I agree it's fine for it to be contextless in general, but in this case we need to either know what job the push relates to when authenticating, or we need to persist a bit of state until we do.
[13:30] <cjwatson> More generalised git access tokens would suggest the latter approach.
[13:30] <wgrant> Indeed, we need more than just the user ID now.
[13:30] <cjwatson> And it's a more standard authn/authz style.
[13:31] <wgrant> The current structure is fine, it's just unfortunate that the opaque data passed between the authn and authz stages is an int.
[13:31] <cjwatson> (OK, we already do authz, it's just that at the moment we can do that with just the Person)
[13:31] <cjwatson> Yeah, maybe I should just uplift that to a JSON dict.
[13:32] <wgrant> AN ASSERTION!
[13:32] <wgrant> Or a string.
[13:32] <wgrant> Either way.
[13:32] <wgrant> I don't think turnip needs to know anything about it, does it?
[13:32] <wgrant> Other than maybe for logging.
[13:32] <cjwatson> Well, yeah, a dict encoded as a string :)
[13:32] <wgrant> So simply saying "any string will do" should work.
[13:32] <cjwatson> No, turnip doesn't need to, though it's also not secret from it.
[13:33] <wgrant> Certainly.
[13:34] <cjwatson> We're already passing around a user/uid pair and only using the uid.
[13:35] <wgrant> Indeed. I'm pretty sure I intended the username as temporary but then we never dropped it once you added the UID.
[13:35] <wgrant> Let's add a third and never drop the other two!
[13:35] <cjwatson> I was going to just go back to the username :)
[13:35] <cjwatson> (I mean, not in general, there was a good reason we switched to UID.)
[13:36] <wgrant> Eh, it needs changes on both ends anyway, so we might as well call it something unconfusing.
[13:37] <cjwatson> It would be an annoying flag-day problem were it not for the fact that authenticateWithPassword is as yet unimplemented.
[13:37] <cjwatson> So we can just declare that it now returns a dict.
[13:37] <wgrant> One but not the other?
[13:37] <wgrant> I thought the existing SSH key method already returned a dict.
[13:37] <cjwatson> And translatePath can be changed to temporarily accept either form.
[13:38] <wgrant> Maybe you changed that but didn't bother with the other.
[13:38] <cjwatson> We pass a params dict to the backend in both cases.
[13:39] <cjwatson> But the SSH backend just picks the username/uid out of its "avatar" directly, since lazr.sshservice deals with the authserver interaction.
[13:39] <wgrant> Oh, SSH auth uses the authserver API
[13:39] <wgrant> Right
[13:39] <wgrant> Had forgotten that split.
[13:39] <wgrant> I suppose we can leave that for now and emulate the dict on the turnip side.
[13:39] <cjwatson> Yeah, that's easy.
[13:40] <wgrant> And sort it out later if we need it, without any serious compat issues.
[13:40] <wgrant> that/it == the SSH side of things, to be clear
[13:40]  * wgrant sleeps.
[13:40] <cjwatson> Yep
[13:40] <cjwatson> Night.
[17:51] <tsimonq2> I plan on writing some code that allows certain email addresses to be subscribed to a packageset (like yakktey-changes but filtered)
[17:51] <tsimonq2> (or so I hope to)
[17:51] <tsimonq2>  1. Is anyone opposed?
[17:52] <tsimonq2>  2. Should I hardcode the email addresses in or should an administrator be allowed to edit the email addresses through a simple web interface?
[17:57] <cjwatson> tsimonq2: no hardcoding
[17:58] <tsimonq2> that's what I thought :P
[17:58] <tsimonq2> cjwatson: but y'all aren't opposed to it?
[17:58] <cjwatson> tsimonq2: it also shouldn't be email addresses IMO, it should be a mechanism to allow people/teams to subscribe to package upload notifications, much as they can to other kinds of notifications
[17:58] <tsimonq2> oh?
[17:58] <cjwatson> I would be opposed to notifying arbitrary addresses, but a subscription mechanism here seems reasonable
[17:59] <tsimonq2> I see, ok
[17:59] <cjwatson> lib/lp/soyuz/mail/packageupload.py would be the place that sends out the mail, but you'd need to figure out how to model the subscriptions
[17:59] <cjwatson> possibly/probably including a way to subscribe to an entire packageset
[18:00] <cjwatson> I'd suggest that it should use/extend the existing structural subscription mechanism
[18:00] <tsimonq2> it would certainly help if there was a page that listed all packages in the Ubuntu archive allowing sorting by packageset
[18:00] <tsimonq2> ok
[18:00] <cjwatson> which doesn't support packagesets, but probably could
[18:01] <cjwatson> well, you don't want to have to subscribe to all the packages individually in any event, so I don't know that such a page would be really the best approach anyway
[18:01] <tsimonq2> well no, it would have checkboxes on the left of each item
[18:01] <cjwatson> firstly, it would be an annoying amount of clicking; secondly, the packageset might change later and you probably want to track that
[18:01] <tsimonq2> then the packageset name would be in bold and if you select that, it selects all the respective packages
[18:01] <cjwatson> if you want to subscribe to a packageset, model that directly, don't record subscriptions to each element of it
[18:02] <tsimonq2> oh yeah good point
[18:02] <tsimonq2> ok
[18:03] <tsimonq2> cjwatson: thanks, I'll get an MP ready within a week or two :)
[18:04] <cjwatson> I think this is likely to be a moderate amount of work involving quite a bit of fiddly hacking, and it will require DB patches, so don't underestimate the work involved.
[18:05] <cjwatson> To begin with given that it touches the DB it will certainly require at least two MPs, probably more.
[18:05] <tsimonq2> yes
[18:05] <tsimonq2> of course
[18:05] <tsimonq2> that's just *my* estimate
[18:05] <tsimonq2> it could be a lot longer
[18:06] <tsimonq2> ok I have to go, I'll look into this later, thanks again!
[18:06] <tsimonq2> win 12
[18:06] <tsimonq2> whoops
[18:07] <cjwatson> I don't want to discourage you, it's just a tough starter project :)
[18:07] <cjwatson> So as long as you're aware of that.
[20:36] <tsimonq2> cjwatson: I am :)