Skip to content

Conversation

@tgxworld
Copy link
Contributor

@tgxworld tgxworld commented Jun 30, 2020

Summary

Support multiple database migrations when running pending migrations.

Also fixes the issue where running pending migrations does not dump the
schema.

Other Information

Related to #34788 and #26542

@tgxworld
Copy link
Contributor Author

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried out Rails::Command.invoke("db:migrate") but kept getting Don't know how to build task 'rails --task' (See the list of available tasks with rails --tasks) when running the railties test suite.

@tgxworld tgxworld force-pushed the fix_pending_migration_not_dumping_schema branch 2 times, most recently from b7de23d to 7344d72 Compare June 30, 2020 04:18
@tgxworld tgxworld changed the title Fix running pending migrations does not dump schema. Support multiple database migrations when running pending migrations. Jun 30, 2020
Comment on lines 137 to 140
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels a little backwards to me. What about updating ActiveRecord::Tasks::DatabaseTasks#migrate to handle multiple databases (or adding a new method, if necessary)? Then the db:migrate Rake task can be refactored to delegate to that method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried out Rails::Command.invoke("db:migrate") but kept getting Don't know how to build task 'rails --task' (See the list of available tasks with rails --tasks) when running the railties test suite.

Rails::Command.invoke("db:migrate") works perfectly in a Rails app but the railties test suite has a different behavior which I can't seem to figure out.

@tgxworld
Copy link
Contributor Author

Test failures don't seem to be related. Seeing it happen on the master branch as well: https://buildkite.com/rails/rails/builds/70429

@tgxworld tgxworld force-pushed the fix_pending_migration_not_dumping_schema branch from 7344d72 to d760e8f Compare June 30, 2020 05:33
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonathanhefner It turns out adding a migrate_all to DatabaseTasks was pretty straight forward.

@tgxworld
Copy link
Contributor Author

Build seems to be consistently broken?

ERROR: error pulling image configuration: Get https://docker-images-prod.s3.amazonaws.com/registry-

I'll retry the build after a few hours.

@hahmed
Copy link
Contributor

hahmed commented Jun 30, 2020

Do we want the migrations to run for all the databases?

I read in a number of PRs that sometimes migrations may be applied to one database but not the other.

@tgxworld
Copy link
Contributor Author

If a migration is created in the migrations path of a particular database, I believe we should run those migrations

@tgxworld tgxworld closed this Jun 30, 2020
@tgxworld tgxworld reopened this Jun 30, 2020
Also fixes the issue where running pending migrations does not dump the
schema.
@tgxworld tgxworld force-pushed the fix_pending_migration_not_dumping_schema branch from d760e8f to 9785b00 Compare July 17, 2020 06:34
@tgxworld tgxworld closed this Aug 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants