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:38 |
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:39 |
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:40 |
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:41 |
cjwatson | I prefer dicts over prefixes. | 00:42 |
wgrant | Great, I marginally prefer them too. | 00:42 |
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:43 |
cjwatson | https://posthere.io/0e1d-4225-836c updated | 00:46 |
wgrant | Fancy. | 00:47 |
wgrant | Do we have revnos easily accessible at that point? | 00:47 |
wgrant | I suspect not. | 00:47 |
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:48 |
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:49 |
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:50 |
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:51 |
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:52 |
wgrant | Well, this wouldn't even require a version bump. | 00:53 |
wgrant | Yay for dicts. | 00:53 |
cjwatson | The "Delivery management web UI" Asana subtask should be done now, right? | 00:54 |
wgrant | Er yeah. | 00:54 |
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:55 |
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:56 |
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:57 |
cjwatson | Adding header/body details? | 00:58 |
cjwatson | Error display is a bit basic too. | 00:58 |
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. | 00:59 |
* cjwatson nods | 01:00 | |
cjwatson | Right, time to crash. Just like buildbot. | 01:00 |
wgrant | Heh | 01:01 |
wgrant | Night | 01:01 |
=== G_ is now known as G | ||
=== G is now known as 32NAAFIPE | ||
=== 32NAAFIPE is now known as G | ||
cjwatson | wgrant: https://code.launchpad.net/~cjwatson/launchpad/testfix-import-warnings/+merge/272228 if you have a sec | 11:40 |
cjwatson | silly me ran bin/test -t archivesubscription rather than -t archive.\*subscription :-P | 11:41 |
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:46 |
wgrant | It is indeed quite obvious. | 11:55 |
blr | morning | 21:01 |
wgrant | Morning. | 23:29 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!