chore: update crate dependencies #509
Conversation
tomyrd
added
the
no changelog
crates/rust-client/Cargo.toml
Outdated
| @@ -35,17 +35,17 @@ lazy_static = { version = "1.5", optional = true } | |||
| miden-lib = { workspace = true } | |||
| miden-objects = { workspace = true } | |||
| miden-tx = { workspace = true } | |||
| prost = { version = "0.12", optional = true, default-features = false, features = ["derive"] } | |||
| prost = { version = "0.13", optional = true, default-features = false, features = ["derive"] } | |||
| rand = { workspace = true } | |||
| rusqlite = { version = "0.31", features = ["vtab", "array", "bundled"], optional = true } | |||
There was a problem hiding this comment.
rusqlite version can't be updated to 0.32 as it is not compatible with the latest version (1.2) of rusqlite_migration.
There was a problem hiding this comment.
I wonder if it's worth maintaining rusqlite_migration since we barely make use of its functionalities right now (basically initialize the DB schema).
There was a problem hiding this comment.
I'm fine with removing them. We can always re-introduce them later as needed.
tomyrd
force-pushed
the
tomyrd-update-dependencies
branch
from
278317c to
685f7cf
Compare
While removing |
There was a problem hiding this comment.
LGTM overall, but I think we should change the way the database is initialized in order to not go through all the CREATE TABLE IF NOT EXISTS statements on every call.
One alternative is to only run store.sql if the config database file does not exist. The other could be to have a single SQL check on the top of the file that avoids running every single statement and checking individually. I think the first one is a bit more solid since it does not require going to the DB at all.
| @@ -100,9 +100,14 @@ impl SqliteStore { | |||
|
|
|||
| /// Returns a new instance of [Store] instantiated with the specified configuration options. | |||
| pub fn new(config: &SqliteStoreConfig) -> Result<Self, StoreError> { | |||
| let mut db = Connection::open(config.database_filepath.clone())?; | |||
| let new_database_file = !Path::new(&config.database_filepath).exists(); | |||
There was a problem hiding this comment.
minor nit: I think I'd name this something like database_exists (and change the value accordingly) to make the code marginally more explicit
There was a problem hiding this comment.
Yeah, you're right. Will change it!
tomyrd
force-pushed
the
tomyrd-update-dependencies
branch
from
1008e1e to
dea1659
Compare
* chore: update dependency versions * fix: builds * chore: remove `rusqlite_migration` dependency * fix: proto field name * fix: check if database exists before intializing
* chore: update dependency versions * fix: builds * chore: remove `rusqlite_migration` dependency * fix: proto field name * fix: check if database exists before intializing
* chore: update dependency versions * fix: builds * chore: remove `rusqlite_migration` dependency * fix: proto field name * fix: check if database exists before intializing
closes #501