chore: Refactor common db capabilities into separate crate#1685
chore: Refactor common db capabilities into separate crate#1685
Conversation
| //! Notice: All of them are infallible. The invariant is a sane content of the database | ||
| //! and humans ensure the sanity of casts. | ||
| //! | ||
| //! Notice: Keep in mind if you _need_ to expand the datatype, only if you require sorting this is | ||
| //! mandatory! | ||
| //! | ||
| //! Notice: Ensure you understand what casting does at the bit-level before changing any. | ||
| //! | ||
| //! Notice: Changing any of these are _backwards-incompatible_ changes that are not caught/covered | ||
| //! by migrations! |
There was a problem hiding this comment.
I really struggled formulating these..
| diesel::sql_query("PRAGMA foreign_keys=ON") | ||
| .execute(conn) | ||
| .map_err(ConnectionManagerError::ConnectionParamSetup)?; | ||
| Ok(()) |
There was a problem hiding this comment.
These pragmas got lost while we added the timeout for all of them. I don't think that's right.
There was a problem hiding this comment.
Sorry I'm not following - those pragmas are all still in configure_connection_on_creation() crates/db/src/manager.rs? Or is that not what you mean?
There was a problem hiding this comment.
Oh, I see, this is just ntx, and it's intentional you drop these?
There was a problem hiding this comment.
Yea ntx builder should use the same consolidated fns as store and validator
drahnr
left a comment
There was a problem hiding this comment.
I don't think the query params as used are unified properly/should be unified but depends on context which one is needed.
Otherwise LGTM
4d7834a to
65696b3
Compare
Mirko-von-Leipzig
left a comment
There was a problem hiding this comment.
I'm still on the fence about doing this before having a concrete plan in mind to evolve this. But lets see how it goes.
Interested to see how ntx-builder goes with the db crate. At that point we will have 3 concrete use cases to design around. |
Context
We have started using the sqlite capabilities found in the store crate in other crates (validator and ntx-builder).
The validator crate currently depends on db-related store code. This is a dependancy that we do not want to maintain.
The ntx-builder crate currently duplicates some db-related code that we can consolidate.
Note that the
apply_migration()fns cannot be consolidated because of theconstnature of the relevant API.Closes #1653.
Changes