Using the connection for migrations

[Razican]

Setup

Versions

• Rust: rustc 1.62.1 (e092d0b6b 2022-07-16)
• Diesel: 2.0.00-rc.1
• Diesel_async: main branch @ 3c9e976
• Database: PostgreSQL
• Operating System: MacOS Monterey

Feature Flags

• diesel: ["postgres_backend", "chrono", "uuid", "numeric", "ipnet-address", "serde_json" ], default features inactive
• diesel_async: ["postgres"]
• diesel_migrations: ["postgres"]

Problem Description

Not really a problem per se, but a missing feature. I would like to not depend on diesel_cli to manage migrations, so I was using diesel_migrations to handle this, but the MigrationHarness needed to retrieve the applied migrations requires the asynchronous connection to implement MigrationConnection, which makes sure that the DB has the correct table.

Unfortunately, this is not yet implemented for asynchronous connections.

What are you trying to accomplish?

I'm trying to embed the migrations with my code, and even create the DB if needed, so that on startup, it will check that the DB is at the correct migration.

What is the expected output?

I would expect to be able to do this with an asynchronous connection, the same way as with a synchronous connection.

What is the actual output?

error[E0277]: the trait bound `AsyncPgConnection: MigrationConnection` is not satisfied
   --> backend/src/lib.rs:152:20
    |
152 |     run_migrations(&mut conn).expect("couldn't run migrations");
    |     -------------- ^^^^^^^^^ the trait `MigrationConnection` is not implemented for `AsyncPgConnection`
    |     |
    |     required by a bound introduced by this call
    |
    = help: the trait `MigrationConnection` is implemented for `PgConnection`
    = note: required because of the requirements on the impl of `MigrationHarness<Pg>` for `AsyncPgConnection`
note: required by a bound in `update_database::{closure#0}::run_migrations`
   --> backend/src/lib.rs:123:31
    |
122 |     fn run_migrations(
    |        -------------- required by a bound in this
123 |         connection: &mut impl MigrationHarness<Pg>,
    |                               ^^^^^^^^^^^^^^^^^^^^ required by this bound in `update_database::{closure#0}::run_migrations`

error[E0277]: the trait bound `AsyncPgConnection: LoadConnection` is not satisfied
   --> backend/src/lib.rs:152:20
    |
152 |     run_migrations(&mut conn).expect("couldn't run migrations");
    |     -------------- ^^^^^^^^^ the trait `LoadConnection` is not implemented for `AsyncPgConnection`
    |     |
    |     required by a bound introduced by this call
    |
    = help: the trait `LoadConnection<B>` is implemented for `PgConnection`
    = note: required because of the requirements on the impl of `diesel::query_dsl::LoadQuery<'_, AsyncPgConnection, MigrationVersion<'static>>` for `SelectStatement<FromClause<diesel_migrations::migration_harness::__diesel_schema_migrations::table>, query_builder::select_clause::SelectClause<diesel_migrations::migration_harness::__diesel_schema_migrations::columns::version>, query_builder::distinct_clause::NoDistinctClause, query_builder::where_clause::NoWhereClause, query_builder::order_clause::OrderClause<diesel::expression::operators::Desc<diesel_migrations::migration_harness::__diesel_schema_migrations::columns::version>>>`
    = note: required because of the requirements on the impl of `MigrationHarness<Pg>` for `AsyncPgConnection`
note: required by a bound in `update_database::{closure#0}::run_migrations`
   --> backend/src/lib.rs:123:31
    |
122 |     fn run_migrations(
    |        -------------- required by a bound in this
123 |         connection: &mut impl MigrationHarness<Pg>,
    |                               ^^^^^^^^^^^^^^^^^^^^ required by this bound in `update_database::{closure#0}::run_migrations`

Are you seeing any additional errors?

No additional error is shown.

Steps to reproduce

    /// Embedded DB migrations.
    const MIGRATIONS: EmbeddedMigrations = embed_migrations!("../migrations");

    /// Runs the migrations in the database.
    #[allow(trivial_casts)]
    fn run_migrations(
        connection: &mut impl MigrationHarness<Pg>,
    ) -> Result<(), Box<dyn Error + Send + Sync + 'static>> {
        let migrations = (&MIGRATIONS as &dyn MigrationSource<Pg>).migrations()?;
        let last = migrations
            .last()
            .expect("could not find the last migration");

        if let Some(last_applied) = connection.applied_migrations()?.first() {
            if last_applied > &last.name().version() {
                // TODO: downgrade, probably add a confirmation parameter in the executable.
                // But how? We would need to get the migrations for downgrade, but we don't have them.
                panic!("Not possible to downgrade the database");
            }
        }

        let _ = connection.run_pending_migrations(MIGRATIONS)?;
        Ok(())
    }

    let mut conn = match db::Connection::establish(db_cfg.conn_str()).await {
        Err(e) => {
            eprintln!("WARNING: error connecting to {}", db_cfg.conn_str());
            db::create(db_cfg)
                .await
                .expect("could not create the database")
        }
        Ok(c) => c,
    };

Checklist

• [x] I have already looked over the issue tracker for similar possible closed issues.

• [x] This issue can be reproduced on Rust's stable channel. (Your issue will be closed if this is not the case)
• [ ] This issue can be reproduced without requiring a third party crate

Suggestion

I can work on this, but, should I add a feature to add this migrations connection implementation, so that it's not active by default for those who don't want to use migrations?

Also, I see that the diesel_migrations trait expects the connection to run the SQL synchronously, so this might be trickier than expected.

[weiznich]

Thanks for opening this bug report. I'm aware of this, but it's good to have an explicit issue for this. I do not consider that to be a huge problem for now, as migrations are mostly run on application start. That's a place where it's perfectly fine to just use a blocking method. I might spend some time to look into ways for fixing this in the future, after the diesel 2.0 release and an initial diesel_async release. (Just to be explicit about that: This takes likely some time, I do not expect to find time for this in the next few months)

[MOZGIII]

One thing to note about the blocking method is it required libpq headers to be present, which is annoying if you bet on async and don't need it otherwise.

---

How would you approach the implementation of this? I presume this will require a rewrite of the diesel_migrations harness to decouple the execution from the control logic somehow. Or would it be better to rewrite the whole harness to be async with the same design but async fns?

[weiznich]

I honestly must say that I do currently not have an idea how to implement this in a composable way. What I can say: It will require quite a bit of design work, which is something I personally do not have the capacity for at the moment.

If your main goal is to remove the dependency on libpq it seems like easier short term solution to just write a rust-postgres based sync connection implementation as third party crate. That shouldn't be to hard based on the existing diesel-async AsyncPgConnection based on tokio-postgres (as that already shows how to do all the type conversions).

[nicrosengren]

Hi,

I created this quickly by just copying the standard diesel migrations and making the strings public. It's not composable in any way. There might be many pitfalls and shortcomings in this solution and we've only used it with postgres but it's been working fine for us this far.

As I understand it it's fine to copy the code as long as I keep the MIT-license.

@weiznich If my understanding regarding the licenses is wrong or there's any other issue with the naming or anything, let me know and I'll correct it. I'm just dropping a link here so that anyone who wants quick migrations can use it.

[weiznich]

@nicrgren It's fine to fork and change that code on your own. I just do not want to do that for diesel-async as well, as I do not want to maintain that much duplicated code.

[RalfJung]

I am staring at https://github.com/weiznich/diesel_async/pull/100 but I don't understand how that fixes this issue. My problem is that I have a AsyncPgConnection and I need to call run_pending_migrations. It seems that with AsyncConnectionWrapper I can take a database URL and turn that into a AsyncConnectionWrapper::<DbConnection>, but that's not useful since I don't have the URL... the connection has long been established (by Rocket, which magically picks up the URL from the config file or environment variable or whatever), I just need to somehow call migrations on it. Help would be appreciated :)

Also, the docs mention a DbConnection type that I cannot find anywhere.

[weiznich]

It expects you to create a new connection either based on the connection url or based on an existing connection via a From impl. The later is only available in an unreleased version yet. I cannot comment on how to do that based on rockets setup as I don't use rocket myself. It might be worth to ask that in their support channels. That written: There is currently no other built-in way that allows you to apply migrations via diesel_migrations at application startup. It's also likely that this won't change anytime soon as diesel_migrations API expects a sync diesel connection. Hopefully that helps at least to understand what's possible and what's not possible.

Also, the docs mention a DbConnection type that I cannot find anywhere.

DbConnection can by any type that implements AsyncConnection.

[RalfJung]

Yeah I think that From instance would suffice. Good to hear this will be resolved in a future version. :)

For now I switched back to sync diesel.