[00:38] <cjwatson> wgrant: Is it worth me pushing up branches for Branch webhooks?  https://posthere.io/0e1d-4225-836c looks vaguely reasonable as a PoC
[00:38] <cjwatson> Very easy on top of your work
[00:39] <wgrant> cjwatson: Branches are trivial, indeed.
[00:39] <cjwatson> Thought I'd see if I could get it to go
[00:39] <wgrant> What is it, like 10 lines of diff?
[00:39] <wgrant> Plus UI
[00:39] <wgrant> Oh, schema.
[00:40] <wgrant> Thinking about what the payload should look like, hmmm./
[00:40] <cjwatson>  11 files changed, 54 insertions(+), 10 deletions(-)
[00:40] <cjwatson> without tests
[00:40] <cjwatson> (ok, a bit of that is local webhook proxy config)
[00:40] <wgrant> Not bad.
[00:41] <wgrant> cjwatson: git:push:0.1 has old and new as dicts, currently containing just the sha1 key.
[00:41] <cjwatson> Right, they could perhaps reasonably become dicts with just "revision_id" keys.
[00:41] <wgrant> We should probably retain consistency, which means changing one of them.
[00:41] <wgrant> I'm not fussed which.
[00:42] <cjwatson> I prefer dicts over prefixes.
[00:42] <wgrant> Great, I marginally prefer them too.
[00:43] <cjwatson> There's an extra level of dict in the git case, but that makes sense given it's a repository hook
[00:43] <wgrant> Right, exactly.
[00:46] <cjwatson> https://posthere.io/0e1d-4225-836c updated
[00:47] <wgrant> Fancy.
[00:47] <wgrant> Do we have revnos easily accessible at that point?
[00:47] <wgrant> I suspect not.
[00:48] <cjwatson> We might, we do have both the db_branch and bzr_branch
[00:48] <cjwatson> I'm doing this from a TipChanged event
[00:48] <wgrant> Eh, let's skip it for now.
[00:48] <wgrant> db_branch isn't entirely trustworthy.
[00:48] <wgrant> And bzr_branch is wrong by that point.
[00:49] <cjwatson> bzr_branch had better not be wrong, it's used to get the new revision ID
[00:49] <wgrant> (well, we could get the info out of bzr_branch in most cases, but not always and it'd always be slow)
[00:49] <wgrant> Get the new revision ID, but not the revno.
[00:49] <wgrant> s/revno/old revno/
[00:49] <cjwatson> We get the old revision ID from db_branch
[00:49] <wgrant> Ah true.
[00:50] <wgrant> Hm
[00:50] <cjwatson> TipChanged.old_tip_revision_id
[00:50] <wgrant> So we could probably include revno reasonably.
[00:50] <cjwatson> It's a Revision query I think?
[00:50] <wgrant> Yep, the number is BranchRevision.sequence
[00:50] <cjwatson> Or is it Branch.revision_count
[00:50] <wgrant> Noooo
[00:50] <wgrant> BranchRevision.sequence is what you want, but you need to be prepared for it to either not be there or not be set.
[00:51] <cjwatson> Maybe that's more trouble than it's worth, given that a reliable webhook should not use the revno
[00:51] <cjwatson> (If there's a race with a push --overwrite, the revno might mean something different by the time the other end runs)
[00:52] <wgrant> True.
[00:52] <wgrant> Easy to add later if we become convinced it's reasonable.
[00:52] <cjwatson> Versioning the format was a good idea.
[00:53] <wgrant> Well, this wouldn't even require a version bump.
[00:53] <wgrant> Yay for dicts.
[00:54] <cjwatson> The "Delivery management web UI" Asana subtask should be done now, right?
[00:54] <wgrant> Er yeah.
[00:55] <cjwatson> Will push this stuff tomorrow.  I could probably attack the remaining subtasks there now that I've gone to the small effort of getting it all working locally ...
[00:55] <cjwatson> Dunno how much you have pending
[00:56] <wgrant> I just use an unrestricted squid in an LXC container for local testing.
[00:56] <cjwatson> I deployed the spec but with a tweak to allow 10.0.0.0/8
[00:56] <cjwatson> Which should perhaps go into devel/deploy
[00:57] <wgrant> Indeed.
[00:57] <wgrant> I meant to go back and make the delivery UI not suck now the infra is there, but have been distracted so far.
[00:58] <cjwatson> Adding header/body details?
[00:58] <cjwatson> Error display is a bit basic too.
[00:59] <wgrant> And making the status clearer without expansion, and adding the status of the latest delivery to each row on +webhooks.
[00:59] <wgrant> And adding refresh capabilities, possibly via dropping the AJAX batchnav stuff and use the delivery API collection directly.
[01:00]  * cjwatson nods
[01:00] <cjwatson> Right, time to crash.  Just like buildbot.
[01:01] <wgrant> Heh
[01:01] <wgrant> Night
[11:40] <cjwatson> wgrant: https://code.launchpad.net/~cjwatson/launchpad/testfix-import-warnings/+merge/272228 if you have a sec
[11:41] <cjwatson> silly me ran bin/test -t archivesubscription rather than -t archive.\*subscription :-P
[11:46] <cjwatson> Actually I'll just self-review that, it's pretty obvious and it's in fact fixing a double-query that was previously present in the view (as shown by the decremented query count)
[11:55] <wgrant> It is indeed quite obvious.
[21:01] <blr> morning
[23:29] <wgrant> Morning.