Expire runs that have no session, just warn while doing so #317
Conversation
Codecov Report
@@ Coverage Diff @@
## master #317 +/- ##
==========================================
+ Coverage 53.21% 53.22% +0.01%
==========================================
Files 100 100
Lines 8624 8628 +4
==========================================
+ Hits 4589 4592 +3
- Misses 3344 3345 +1
Partials 691 691
Continue to review full report at Codecov.
|
| ContactID models.ContactID `db:"contact_id"` | ||
| RunID models.FlowRunID `db:"run_id"` | ||
| ParentUUID *flows.RunUUID `db:"parent_uuid"` | ||
| SessionID *models.SessionID `db:"session_id"` |
There was a problem hiding this comment.
don't we generally model null ids like NilSessionID = SessionID(0) ?
There was a problem hiding this comment.
We do both in various places. I'm not positive what the right thing is but I've kind of settled that if the TYPE can be null normally, then having a nullable type derived from null.Int makes sense, but if not then selecting to a pointer is more correct. IE, within Mailroom are we ever "setting" null.
In this case SessionID being nil is very much the exception and this is the only places that looks at that possibility so using a pointer made sense to me. We get to keep the "stricter" type elsewhere. (and have things fail if we try to select a NULL into that stricter type)
Happy to change if you disagree.
New WhatsApp BSP channel: 360dialog
Explored our DB for old runs that weren't archived / deleted and found that those that were sticking around were runs that no longer had a session. It isn't super clear at this point what the root cause is there, from the RapidPro session culling code we only cull sessions that have ended, so this points to events where somehow we update a session without updating a flowrun, which I don't see any possibility for on the Mailroom side.
In any case, once we are in this state there's nothing to do be done. So this change moves forward with expiring these runs but warns to sentry about it so we can continue to track what may be occurring in updating the flow runs while still building valid archives.