Fix sqlite memory connection issues #2104
Conversation
Signed-off-by: Cristian Le <[email protected]>
|
Hey! Do you have an example run in our CI where this fails? According to this comment, below the one you linked, this issue should be fixed in SQLx: launchbadge/sqlx#362 (comment) I've just ran our unit tests 100x both on mac and linux, and on neither platform can I replicate a failure. |
|
Not in the local CI because the system
Yes this is an upstream issue. The fix is relatively simple to incorporate here though until these fixes are found over there. |
There was a problem hiding this comment.
A couple of comments, though I'm not sure this change is needed.
EG, this approach suggests using a named in memory file: launchbadge/sqlx#2510 (comment)
Which it looks like they already do: https://github.com/launchbadge/sqlx/blob/1388fc8acc07b41ab38f4dc096f03ba9c7b851f8/sqlx-sqlite/src/options/parse.rs#L18-L22
| @@ -122,7 +135,7 @@ impl SqliteStore { | |||
| impl Store for SqliteStore { | |||
| async fn push_batch( | |||
| &self, | |||
| records: impl Iterator<Item = &Record<EncryptedData>> + Send + Sync, | |||
There was a problem hiding this comment.
Any reason for the formatting change?
There was a problem hiding this comment.
Just my IDE being overzealous, I will fix them shortly
| @@ -72,16 +85,16 @@ impl SqliteStore { | |||
| "insert or ignore into store(id, idx, host, tag, timestamp, version, data, cek) | |||
| values(?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)", | |||
| ) | |||
| .bind(r.id.0.as_hyphenated().to_string()) | |||
I'm afraid we do not support using the system sqlite. If you can replicate the failure with our lockfile, then I'm happy to merge the fixes. Please don't upstream any more changes unless they are fixing a failure that can be replicated with our build. |
|
Also, I know I linked you it before, but it looks like you need to enable Void, having the same issue: https://forum.atuin.sh/t/packaging-atuin-for-void-linux/20/6?u=ellie |
Basically for in-memory sqlite databases it can happen that the pool sharing is not handled properly and the database overwrites the old one. Since the in-memory databases are used only for testing it should be fine to simply ensure the connection lives forever. Thoughts?
Resolves #2073
Reference:
Checks