softprops / afterparty

rust github webhook server
MIT License
55 stars 13 forks source link

Keep state across webhook deliveries #18

Open ms705 opened 7 years ago

ms705 commented 7 years ago

In Taster, I use afterparty to provide a regression testing service for Rust applications. As part of this, I need to keep some state across multiple webhook deliveries (namely, performance results from the previous delivery).

Unfortunately, the Hook implementation requires the closure passed to handle to be Fn, not FnMut, so I cannot have any mutable captures. However, such captures are required so that I can actually persist results across invocations.

Is this a restriction that could possibly be relaxed? I don't see any other way of implementing this use case.

(Likewise, I would like to use the closure passed to handle to only push a request to do some testing onto a work queue, rather than doing all the work before returning from the request, since GitHub keeps thinking that my webhooks time out when the testing takes a while. However, not being able to mutate anything from within the closure makes this impossible, too.)

(Cc @jonhoo)

jonhoo commented 7 years ago

The workaround for this is presumably to use an Arc<Mutex>, which you use to get mutable access even from a Fn. It's not terribly clean, but it'll work even if it turns out that a Fn is indeed necessary. I suspect the reason for the Fn requirement is that afterparty allows concurrent requests to its endpoints (if GitHub notifies you about two pushed at the same time for example), which fundamentally requires being able to call the function even if you only have a read-only borrow of it.

softprops commented 7 years ago

So fn is provided out of the box for convenience but you could also try. impl Hook for YourType and have a type specific to your application.

I've considered spawning a thread to fire hooks but I err'd on this side of letting users choose there own strategy because there a growing number of options out there and didn't want to be perspriptive. I also wanted to keep transitive dependencies low(er). All github requires is a 200 response. You could may maybe do a fire and forget hook that pushes deliveries to a channel and consume from the other end on a separate thread. There's a lot of options there so I didn't want to be prescriptive.

softprops commented 7 years ago

BTW. Thanks for sharing that link I'm going to check out your project!