Copy/pasting the Slack convo for context and some ideas:
Gordon@Yuval Shavit maybe some kind of CD for ^ for when multiple people are commenting at the same time
Yuval Shavit
CD?
Gordon
sorry, CoolDown - a timer that disallows similar actions until it has elapsed
Yuval Shavit
ah, gotcha. Yeah, the problem is that in a lot of cases you don’t want that. If I post a question and then you post an answer very quickly, and then I say “ah cool, makes sense” — then I think you probably do want to see that ack.
Definitely not opposed to it (and I even called out this kind of notification-spam in my short doc on comment notifications), but I think it needs some thought as to what the desired behavior is.
Gordon
I guess it depends on the scenario - I use the slack comments to be like "hey, look at the PR again". If there's a lot of back-and-forth, I'm most likely already looking at it
Yuval Shavit
Yep, I agree completely — the trick is when is it “a lot”, as opposed to “ask a quick question, get a quick response.”
I’m definitely not saying we can’t draw the line — just that I haven’t yet put much thought into where that line is.
Gordon
Yeah, I think it's 1x back-and-forth = :+1: , > a couple (2?) then start to throttle messages
Gordon
some kind of heuristic like that
Yuval Shavit
Some of the nicer heuristics might require new klotho functionality, and specifically the ability to set a delay on subsub notifications. That would let us take the notifications and put them on a queue to be processed in 30 seconds (or whatever), and in the meanwhile we’ll have a better sense of the history
Gordon
oh, maybe instead of "last commenter" it could do something more like batching (after the first message, to keep it responsive for the quick case)
Yuval Shavit
yeah, that batching is exactly the kind of thing I mean :slightly_smiling_face:
Gordon
so after the first one, wait until no comments for X minutes then a
gordon-klotho commented Y times on the PRYuval Shavit
yup, :100:
Gordon
well, doesn't have to be a queue per se just some data store that we can add & remove from
Yuval Shavit
I actually think that would not only be a great feature for the bot, but also a great way to incorporate a new capability into the app!
Gordon
like a DB :)
Yuval Shavit
how would you implement the “[wait] X minutes then [action]“?
Gordon
polling
Yuval Shavit
yuck :stuck_out_tongue:
Gordon
why yuck?
Yuval Shavit
well firstly it’ll be a good deal more expensive — it means we need to use fargate instead of lambda. Secondly, it forces an infra decision (fargate instead of lambda). And thirdly, event-driven is just nicer imo.
Gordon
Well, you can add polling to lambda, just not through Klotho (yet)
Gordon
Event-driven is okay, but how do you "reset" the timer when you get a new message? There's not really a clean way to do that in event-driven
Yuval Shavit
as in have one fire off every 1 minute or so to collect all the actions you need? yeah, you could do that.
Gordon
event-driven could actually have worse perf depending on how rapidly the commenting happens (say 1 comment per 30s vs polling every 5 minutes)
Yuval Shavit
I was thinking that you’d put the info into the DB right away, and then a message to the queue to basically say “check back on this in 30 seconds.”
So ultimately, it’s still polling, just with a different mechanism.
Yuval Shavit
so I retract my yuck. point is, it would require some functionality that klotho doesn’t yet have :slightly_smiling_face:
Gordon
would be a good impetus for implementing such
Yuval Shavit
agreed!
Copy/pasting the Slack convo for context and some ideas: