Final event dispatcher tweaks as pre-requisites for non-blocking #6795
Conversation
... instead of using unnamed tuples and long parameter lists.
This will allow us to output warnings if the (non-blocking) delivery gets too far behind, because we can tell how long it took between enqueuing the event and actually sending it. This commit adds another migration to said database, so I slightly refactored the migration code.
benjamin-stacks
requested review from
aaronb-stacks,
brice-stacks and
hstove-stacks
| Ok((name, col_type)) | ||
| })?; | ||
|
|
||
| let mut payload_is_blob = None; |
There was a problem hiding this comment.
Would it be possible to modify the database to actually store a database number now?? cause then you just would check "If version exists, use version, if not, its database schema 0?" Unless we don't anticipate any further database changes in which case maybe this is just fine, but I could see this getting needlessly complex.
There was a problem hiding this comment.
I have no idea if this DB is going to change much in the future. I also wasn't sure there should really be a one-off migration mechanism for this one DB; it seems like that should be shared functionality.
That's why I went with the compromise: Split out the detection of the current migration version from the migration itself. Since that detection is contained completely inside get_schema_version, it would make it easy to later replace it with a DB-stored identifier.
There was a problem hiding this comment.
Yeah, I think we might as well version it.
Codecov Report❌ Patch coverage is
❌ Your project check has failed because the head coverage (72.39%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #6795 +/- ##
===========================================
- Coverage 77.77% 72.39% -5.39%
===========================================
Files 585 585
Lines 361916 362030 +114
===========================================
- Hits 281498 262089 -19409
- Misses 80418 99941 +19523
... and 279 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
brice-stacks
left a comment
brice-stacks
left a comment
There was a problem hiding this comment.
This looks good. Just noticed one typo and I like the idea of adding a version to the DB
| } | ||
|
|
||
| fn get_payload_column_type(&self) -> Result<Option<String>, db_error> { | ||
| /// The initial schema of the datebase when this code was first created |
| Ok((name, col_type)) | ||
| })?; | ||
|
|
||
| let mut payload_is_blob = None; |
There was a problem hiding this comment.
Yeah, I think we might as well version it.
3 participants
I recommend commit-by-commit review.
a48a100: uses a less brittle mechanism to find out what the id of the just-inserted DB record is41717b4: just some refactoring that bundles multiple individual parameters into a single struct0d62bd4: add a time stamp column to the event observer DB so we know when an event payload was stored