Postdeploywebhook moves the ShouldSendFinishMessage

[lilida]

This is a fix trying to address the issue below: https://github.com/pinterest/teletraan/issues/517

It tries to consolidate when to send a deploy finish message. FinishNotifyJob was used to send slack notification today

[lilida]

There are five users and I will follow up

[sbaogang]

Sorry for the late reply, from what I'm reading -

Invoking webhook is one-time only, right after deploy succeeds the first time; sending notification on the other hand is a repeating work happens for every "transition" call.

For example, in the auto-scaling case where hosts keep getting terminated and new machine keep getting added, let us assume all the newly added hosts having issues with deploy in the beginning (maybe can not warm up in time) but later on succeed. In this case, we would send notify in the pattern of FAILING->SUCCEEDING->FAILING->SUCCEEDING... If we change the behavior, we would call the hook over and over again, I do not think this is most people anticipated.

webhook checking is closely related to the check of (success/total>threshold && sucdate==null); and send notify is related to STATE changes, regardless of success or fail. That might be the reason why the code was written in the current way.

But regardless where these logic live, I think calling webhook once makes more sense to most people, in most case. I would not recommend to change such behavior.