-
Notifications
You must be signed in to change notification settings - Fork 21.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rails 7.1 changes established database connection after calling rails db:test:prepare
for multi-database apps
#51830
Comments
This fixes it but I don't think it's the direction we want to go in: justinko@2fc0ded I think there's some confusion about responsibility between |
@justinko I think a problem with that approach is that those tasks already call
|
Which is essentially idempotent/no-op as Not saying this is the correct approach but we do need to find a clean way to restore to the original db config. |
Steps to reproduce
rails new example
In the new app, setup a multiple-database configuration
config/database.yml
file:Add the following to the beginning of
db/seeds.rb
:Run
rails db:migrate
to create the schema files.Run
RAILS_ENV=test rails db:drop db:test:prepare db:seed
Expected behavior
In Rails 7.0 and prior, the
seed
task would run against the default,primary
database connection, which is what I would expect. This can be seen by the output of this example app on Rails 7.0:Actual behavior
In Rails 7.1+, the
seed
task runs against the last database connection defined, which may cause seeding to break. Here is the same output from this type of multi-database example app in Rails 7.1:Note that now the seeding process is trying to run while connected to the secondary
animals
database. In a more real-life scenario, this may cause seeding to fail, since seeds may not work if running against a non-primary database where your model tables may not exist.You can also see how it's the
db:test:prepare
task that is affecting the active connection, since if you calldb:seed
by itself in Rails 7.1, it does still connect to the expected primary database. So this issue is specific to chaining multiple tasks in a single invocation afterdb:test:prepare
.System configuration
Rails version: 7.1.3.2
Ruby version: 3.3.1
Additional notes
db:test:purge
task (which is called as a dependency ofdb:test:prepare
) loops over the database connections and leaves the app connected to whatever the last connection to be looped over was: https://github.com/rails/rails/blob/v7.1.3.2/activerecord/lib/active_record/railties/databases.rake#L559-L563RAILS_ENV=test rails db:drop db:test:prepare && RAILS_ENV=test rails db:seed
instead, then the seed task does execute against the expected primary database, but this means tasks are working different when called in isolation versus being chained.db:test:prepare
in the same CLI invocation, since in all cases, it leaves the active database connection as the last one defined for multi-database apps.db:test:prepare
would specifically reset the connection back to the default connection: https://github.com/rails/rails/blob/v7.0.8.1/activerecord/lib/active_record/railties/databases.rake#L559-L563db:test:purge
doesn't use the newwith_temporary_connection_for_each
approach (which does help avoid these type of problems). But I'm note entirely certain it can use the new approach, since on a quick test, I ran into errors, since the database doesn't exist yet in my example case (due to thedb:drop
). But there might be other better ways to leverage the new setup for this specific task.ActiveRecord::Base.establish_connection(ActiveRecord::Tasks::DatabaseTasks.env.to_sym)
at the end of thedb:test:purge
task to reset any connections the loop may have established. That worked in a quick test, but again, I'm unsure if that's necessarily the best solution.The text was updated successfully, but these errors were encountered: