[MM-62410] Automatically cleanup stale calls in RTCD enabled deployments #940
Conversation
| if err != nil { | ||
| m.ctx.LogDebug("failed to get version info", "err", err.Error(), "callID", call.ID, "rtcdHost", call.Props.RTCDHost) | ||
| return false |
There was a problem hiding this comment.
This defaults to call is still running if there's a problem getting the version info -- will that cause trouble? Should we return the err and have logic for it?
There was a problem hiding this comment.
A stuck call is strictly better than killing a live call, so unless we get a guarantee from the RTCD side that the call has ended, we err on the side of caution.
|
|
||
| if code != http.StatusOK && code != http.StatusNotFound { | ||
| m.ctx.LogDebug("unexpected status code from RTCD", "code", code, "callID", call.ID, "rtcdHost", call.Props.RTCDHost) | ||
| // The request above could fail for various reasons, in which case we can't assume the call has ended. |
There was a problem hiding this comment.
ah, this confirms the default behavior. Maybe add this comment to the above as well? (would just save some cognitive overhead)
| }) | ||
| require.NoError(t, err) | ||
|
|
||
| mockAPI.On("LogDebug", "cleaning up calls state", |
There was a problem hiding this comment.
this must have been a pain to get exactly right, but I kind of like that the tests have the logs laid out like this -- you can see exactly what's supposed to happen. It's easy to understand.
There was a problem hiding this comment.
Yeah, it takes some time, but it ensures the tests are solid, even in the presence of heavy mocking.
cpoile
added
3: Reviews Complete
Summary
PR adds logic to automatically clean up stale calls/sessions on plugin activation under RTCD-backed deployments. This is achieved by asking the RTCD side if the calls are still ongoing.
Related PR
mattermost/rtcd#166
Ticket Link
https://mattermost.atlassian.net/browse/MM-62410