ankane/multiverse

Does `multiverse` raise a `PendingMigration` error if not migrated?

Closed this issue · 6 comments

For instance, in a vanilla Rails app if you attempt to start the app with migrations pending, Rails throws PendingMigrationError, reminding you to run rake db:migrate.

From what I've tried so far, it does not seem like the current behaviour supports that -- the app does not care if there are pending migrations in other DBs.

Was wondering if such a thing exists for multiverse or if there's a plan towards that?

@edisonywh and I have looked into this, and came up with a solution for our app. (Rails 4.2.11)

We looked into how Rails does it (inserting the migration checker into the middleware), and figured we could do the same:

Rails.application.config.middleware.insert_after '::ActionDispatch::Callbacks', 'ActiveRecord::Migration::CheckPendingCustom'

@ankane What do you think? Perhaps we could open a PR for this?

Hey @edisonywh and @jooeycheng, you're right that Multiverse doesn't support this right now. Feel free to post the solution you come up with. Since Multiverse will be phased on with Rails 6, I don't think it's worth the effort for a PR.

Although Multiverse is no longer useful in Rails 6 onwards, I think there are still quite a number of people who will still be on Rails 5 and below. I don't mind creating a PR for this (also allows us to upstream our fix). But I'm not sure how to write an elegant solution and a test for this. 🙈

Anyway, here's what we did - in our app, we have have Rails Engine which connects to a different database with multiverse.

With inspiration from active_record/migration.rb, this is what we came up with:

  1. Create a CheckPendingMigration class. For our case, we're using Rails Engine, hence we created the file on the Engine bcos it's sort of the Engine's domain.
# (my_engine) ./app/utilities/my_engine/check_pending_migration.rb
#
# we use `rake db:task` instead of `rails db:task` bcos we're on Rails 4 (yes, we're old)

module MyEngine
  class PendingMigrationError < ActiveRecord::MigrationError
    def initialize
      super("Migrations are pending. To resolve this issue, run:\n\n\tbundle exec rake db:migrate:all")
    end
  end

  class CheckPendingMigration
    def initialize(app)
      @app = app
    end

    def call(env)
      if connection.supports_migrations?
        raise MyEngine::PendingMigrationError if ActiveRecord::Migrator.needs_migration?(connection)
      end
      @app.call(env)
    end

    private

    def connection
      MyEngine::CustomDatabase.connection
    end
  end
end
  1. Then, append the CheckPendingMigration class into the "Rails middleware" on app initialize (this is how Rails does it)
# ./app/config/initializers/check_pending_migration.rb

if Rails.env.development? || Rails.env.test?
  Rails.application.config.middleware.insert_after 'ActionDispatch::Callbacks', 'MyEngine::CheckPendingMigration'
end
  1. Notice the db:migrate:all task in step 1. That's custom:
# ./lib/tasks/db.rake

namespace :db do
  namespace :migrate do
    task all: :environment do
      puts 'Migrating main database'
      Rake::Task["db:migrate"].invoke

      %w[custom_db1 custom_db2].each do |db|
        puts "Migrating #{db} database with multiverse"
        `DB=#{db} bundle exec rake db:migrate`
      end
    end
  end
end
  1. If you're using Capistrano for deployments. Refer to #28.

Notice we prompt rake db:migrate:all instead of DB=custom rake db:migrate. This is due a bug in ActiveRecord::Migrator.needs_migration?(connection). It doesn't respect the given connection, and takes the default ActiveRecord::Base.connection. So, we're not able to check if migrations are pending on our custom database. We added a db:migrate:all command to always attempt to migrate all databases instead.

Explanation for the bug mentioned above:
See active_record/migration.rb and active_record/schema_migration.rb. Notice SchemaMigration inherits from ActiveRecord::Base, so SchemaMigration checks will always run on the default ActiveRecord::Base.connection, and only check for pending migrations on that connection. This has since been refactored to work differently in Rails 6.

Hey @jooeycheng, thanks for sharing. Seems like it should be enough for other developers to add to their app if desired. As I mentioned before, I don't think it's worth the effort for a PR.

Got it @ankane. Thanks!

I think we can consider this issue closed.

Sounds good, also added a note in the readme about this limitation.