Refactor migration to move migrations paths to connection #31727

eileencodes · 2018-01-17T19:31:29Z

Rails has some support for multiple databases but it can be hard to
handle migrations with those. The easiest way to implement multiple
databases is to contain migrations into their own folder ("db/migrate"
for the primary db and "db/seconddb_migrate" for the second db). Without
this you would need to write code that allowed you to switch connections
in migrations. I can tell you from experience that is not a fun way to
implement multiple databases.

This refactoring is a pre-requisite for implementing other features
related to parallel testing and improved handling for multiple
databases.

The refactoring here moves the class methods from the Migrator class
into it's own new class MigrationContext. The goal was to move the
migrations_paths method off of the Migrator class and onto the
connection. This allows users to do the following in their
database.yml:

development:
  adapter: mysql2
  username: root
  password:

development_seconddb:
  adapter: mysql2
  username: root
  password:
  migrations_paths: "db/second_db_migrate"

Migrations for the seconddb can now be store in the
db/second_db_migrate directory. Migrations for the primary database
are stored in db/migrate".

The refactoring here drastically reduces the internal API for migrations
since we don't need to pass migrations_paths around to every single
method. Additionally this change does not require any Rails applications
to make changes unless they want to use the new public API. All of the
class methods from the Migrator class were nodoc'd except for the
migrations_paths and migrations_path getter/setters respectively.

cc/ @tenderlove

tenderlove

LGTM. I think we can remove one more migrations_paths parameter, but this is good to merge as-is IMO.

tenderlove · 2018-01-17T19:33:47Z

activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb

@@ -1049,8 +1049,8 @@ def assume_migrated_upto_version(version, migrations_paths)
        sm_table = quote_table_name(ActiveRecord::SchemaMigration.table_name)

        migrated = ActiveRecord::SchemaMigration.all_versions.map(&:to_i)
-        versions = ActiveRecord::Migrator.migration_files(migrations_paths).map do |file|
-          ActiveRecord::Migrator.parse_migration_filename(file).first.to_i
+        versions = migration_context.migration_files(migrations_paths).map do |file|


Can we eliminate the migration_paths parameter here? I think migration_context contains the same information.

Doh I missed that one.

Rails has some support for multiple databases but it can be hard to handle migrations with those. The easiest way to implement multiple databases is to contain migrations into their own folder ("db/migrate" for the primary db and "db/seconddb_migrate" for the second db). Without this you would need to write code that allowed you to switch connections in migrations. I can tell you from experience that is not a fun way to implement multiple databases. This refactoring is a pre-requisite for implementing other features related to parallel testing and improved handling for multiple databases. The refactoring here moves the class methods from the `Migrator` class into it's own new class `MigrationContext`. The goal was to move the `migrations_paths` method off of the `Migrator` class and onto the connection. This allows users to do the following in their `database.yml`: ``` development: adapter: mysql2 username: root password: development_seconddb: adapter: mysql2 username: root password: migrations_paths: "db/second_db_migrate" ``` Migrations for the `seconddb` can now be store in the `db/second_db_migrate` directory. Migrations for the primary database are stored in `db/migrate`". The refactoring here drastically reduces the internal API for migrations since we don't need to pass `migrations_paths` around to every single method. Additionally this change does not require any Rails applications to make changes unless they want to use the new public API. All of the class methods from the `Migrator` class were `nodoc`'d except for the `migrations_paths` and `migrations_path` getter/setters respectively.

This was added in #31727, but it is unused.

andrew-aladev · 2018-02-01T17:36:16Z

@matthewd, this PR is not related to maintaining multiple connections for multiple databases. Migrations are still working with ActiveRecord::Base.connection. They are applied one-by-one. It is not possible to apply migration when two connections are running.

bogdan · 2018-06-06T09:56:05Z

I know this is too late, but maybe it will be helpful.
The convention explained here is weird:

development:
  adapter: mysql2
  username: root
  password:

development_seconddb:
  adapter: mysql2
  username: root
  password:
  migrations_paths: "db/second_db_migrate"

I'd have to duplicate migrations_path between different envs and the <env>_<db-name> convention feels redundant to me.
I would prefer the following format instead:

main:
  migrations_path: 'db/migrate'
  development:
    adapter: mysql2
    username: root
    password:
  test:
    ...
  production
    ...
seconddb:
  migrations_paths: "db/second_db_migrate"
  development:
    adapter: mysql2
    username: root
    password:
  test:
    ....
  production:
    ...

eileencodes · 2018-06-07T12:06:31Z

@bogdan - it was just an example before I realized there was a three tier config. Both work.

The API for invoking migrations has changed in rails/active record 5.2 see rails/rails#31727

The API for invoking migrations has changed in rails/active record 5.2 see rails/rails#31727 Leave support for old migration API so we can run tests against old versions of ActiveRecord to check compatibility

`ActiveRecord::Migrator.migrations_path=` was deprecated in rails#31727. This commit fixes: - `ActiveRecord::Migrator.migrations_path=` is deprecated, but not `ActiveRecord::Migrator.migrations_paths=` - Adds missing space The warning including this commit: ``` DEPRECATION WARNING: `ActiveRecord::Migrator.migrations_path=` is now deprecated and will be removed in Rails 6.0. You can set the `migrations_paths` on the `connection` instead through the `database.yml`. ``` Since it was deprecated in Rails 5.2 we should backport it to the `5-2-stable` branch. Related to rails#31727

…pto_version` Since #31727, `migrations_paths` in `assume_migrated_upto_version` is no longer used.

eileencodes added the activerecord label Jan 17, 2018

eileencodes self-assigned this Jan 17, 2018

eileencodes force-pushed the refactor-migration-classes-to-allow-for-migrations_paths-on-conn branch from 6433703 to e881a62 Compare January 17, 2018 19:33

tenderlove approved these changes Jan 17, 2018

View reviewed changes

eileencodes force-pushed the refactor-migration-classes-to-allow-for-migrations_paths-on-conn branch 2 times, most recently from a24a82d to e5fdc19 Compare January 17, 2018 20:00

rafaelfranca approved these changes Jan 17, 2018

View reviewed changes

eileencodes force-pushed the refactor-migration-classes-to-allow-for-migrations_paths-on-conn branch from e5fdc19 to 9b7e564 Compare January 17, 2018 20:52

eileencodes force-pushed the refactor-migration-classes-to-allow-for-migrations_paths-on-conn branch from 9b7e564 to a2827ec Compare January 18, 2018 13:55

eileencodes merged commit 6e74e1d into rails:master Jan 18, 2018

eileencodes deleted the refactor-migration-classes-to-allow-for-migrations_paths-on-conn branch January 18, 2018 14:49

kamipo added a commit that referenced this pull request Jan 19, 2018

Remove unused migration_context in DatabaseTasks

fa17fcc

This was added in #31727, but it is unused.

kamipo mentioned this pull request Jan 19, 2018

Deprecate passing a connection to Migration.check_pending! #28294

Closed

fatkodima mentioned this pull request Jan 31, 2018

Fix migrations in tests for rails 5.2 collectiveidea/audited#409

Merged

matthewd mentioned this pull request Feb 1, 2018

active record: connection to multiple databases in different models #31857

Closed

rafaelfranca mentioned this pull request Feb 17, 2018

Test on Rails 5.2.0.rc1 heartcombo/devise#4711

Merged

phildionne mentioned this pull request Apr 26, 2018

Engine support ankane/multiverse#12

Closed

davinlagerroos pushed a commit to umn-asr/iron_fixture_extractor that referenced this pull request Jun 14, 2018

Add support for running tests > ActiveRecord 5.2

648300d

The API for invoking migrations has changed in rails/active record 5.2 see rails/rails#31727

bogdanvlviv mentioned this pull request Sep 17, 2018

Fix deprecation warning of ActiveRecord::Migrator.migrations_path= #33902

Merged

agrare mentioned this pull request Dec 19, 2018

Workaround change to needs_migration? in rails 5.2 RedHatInsights/topological_inventory-api#76

Closed

kamipo added a commit that referenced this pull request Dec 28, 2018

Deprecate passing migrations_paths to `connection.assume_migrated_u…

c1b14ad

…pto_version` Since #31727, `migrations_paths` in `assume_migrated_upto_version` is no longer used.

jooeycheng mentioned this pull request Apr 14, 2019

Does multiverse raise a PendingMigration error if not migrated? ankane/multiverse#31

Closed

p8 mentioned this pull request Aug 11, 2021

PendingMigrationError not raised for engine migrations #41460

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor migration to move migrations paths to connection #31727

Refactor migration to move migrations paths to connection #31727

eileencodes commented Jan 17, 2018

tenderlove left a comment

tenderlove Jan 17, 2018

eileencodes Jan 17, 2018

andrew-aladev commented Feb 1, 2018

bogdan commented Jun 6, 2018 •

edited

eileencodes commented Jun 7, 2018

Refactor migration to move migrations paths to connection #31727

Refactor migration to move migrations paths to connection #31727

Conversation

eileencodes commented Jan 17, 2018

tenderlove left a comment

Choose a reason for hiding this comment

tenderlove Jan 17, 2018

Choose a reason for hiding this comment

eileencodes Jan 17, 2018

Choose a reason for hiding this comment

andrew-aladev commented Feb 1, 2018

bogdan commented Jun 6, 2018 • edited

eileencodes commented Jun 7, 2018

bogdan commented Jun 6, 2018 •

edited