WIP: Implement Diesel MultiConnect #6279
Conversation
Signed-off-by: BlackDex <[email protected]>
|
|
||
| #[allow(non_camel_case_types)] | ||
| pub enum DbConnInner { $( #[cfg($name)] $name(PooledConnection<ConnectionManager< $ty >>), )+ } | ||
| #[cfg(query_logger)] |
There was a problem hiding this comment.
You can likely also replace the query logger dependency by setting the right instrumentation for either the connection or at global level. That can possibly help simplify the code a bit more.
There was a problem hiding this comment.
I did noticed yesterday during a test that i was missing the query parameters. But i haven't verified if this was because of the diesel_logger crate or something else.
I was looking into this though, i tried to also extract the file and line of where the call was placed, but that is a bit difficult hehe.
| #[derive(diesel::MultiConnection)] | ||
| pub enum DbConnInner { | ||
| #[cfg(sqlite)] | ||
| Sqlite(diesel::sqlite::SqliteConnection), |
There was a problem hiding this comment.
#If you use the provided establish method it is rather important to put the SQLite variant as last entry. The implementation of the establish is rather dumb, it just tries to connect to each variant in order and as SQLite accepts almost anything as database url that might cause issues connecting to the other backends. The other and likely better solution is to check the db url and construct the correct enum variant directly b calling establish on the inner connection. That should give you also better runtime error messages if something goes wrong.
| Self::mysql => String::new(), | ||
| Self::postgresql => String::new(), | ||
| #[cfg(sqlite)] | ||
| Self::Sqlite => "PRAGMA busy_timeout = 5000; PRAGMA synchronous = NORMAL;".to_string(), |
There was a problem hiding this comment.
It might be meaningful to use the other settings from here as well?
| pub async fn backup_database(conn: &mut DbConn) -> Result<String, Error> { | ||
| db_run! {@raw conn: | ||
| pub async fn backup_database(conn: &DbConn) -> Result<String, Error> { | ||
| db_run! {conn: |
There was a problem hiding this comment.
For the backup implementation this function might be more appropriated.
Thanks! Ill take a look at them, and adjust where needed. |
A Restart of a previous attempt to use the Diesel MultiConnection derive.
Still a work-in-progress, as Diesel needs some patches before this can be built. (See diesel-rs/diesel#4751)