aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authoreileencodes <eileencodes@gmail.com>2019-06-26 14:25:11 -0400
committereileencodes <eileencodes@gmail.com>2019-06-27 09:54:25 -0400
commitdf6b0de7d9522c46c140c556ee61e09df14ef97a (patch)
treeb343408225debc541b0ef56f3c24312e5d7f4935
parentb65f88652f835093030bf65b9e31e727ba58a6de (diff)
downloadrails-df6b0de7d9522c46c140c556ee61e09df14ef97a.tar.gz
rails-df6b0de7d9522c46c140c556ee61e09df14ef97a.tar.bz2
rails-df6b0de7d9522c46c140c556ee61e09df14ef97a.zip
Load initial database.yml once, and warn if we can't create tasks
For multiple databases we attempt to generate the tasks by reading the database.yml before the Rails application is booted. This means that we need to strip out ERB since it could be reading Rails configs. In some cases like https://github.com/rails/rails/issues/36540 the ERB is too complex and we can't overwrite with the DummyCompilier we used in https://github.com/rails/rails/pull/35497. For the complex causes we simply issue a warning that says we couldn't infer the database tasks from the database.yml. While working on this I decided to update the code to only load the database.yml once initially so that we avoid having to issue the same warning multiple times. Note that this had no performance impact in my testing and is merely for not having to save the error off somewhere. Also this feels cleaner. Note that this will not break running tasks that exist, it will just mean that tasks for multi-db like `db:create:other_db` will not be generated. If the database.yml is actually unreadable it will blow up during normal rake task calls. Fixes #36540
-rw-r--r--activerecord/lib/active_record/railties/databases.rake16
-rw-r--r--activerecord/lib/active_record/tasks/database_tasks.rb13
-rw-r--r--railties/test/application/rake/dbs_test.rb28
3 files changed, 48 insertions, 9 deletions
diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake
index d17acc408c..648fdd0dc4 100644
--- a/activerecord/lib/active_record/railties/databases.rake
+++ b/activerecord/lib/active_record/railties/databases.rake
@@ -2,6 +2,8 @@
require "active_record"
+databases = ActiveRecord::Tasks::DatabaseTasks.setup_initial_database_yaml
+
db_namespace = namespace :db do
desc "Set the environment value for the database"
task "environment:set" => :load_config do
@@ -23,7 +25,7 @@ db_namespace = namespace :db do
ActiveRecord::Tasks::DatabaseTasks.create_all
end
- ActiveRecord::Tasks::DatabaseTasks.for_each do |spec_name|
+ ActiveRecord::Tasks::DatabaseTasks.for_each(databases) do |spec_name|
desc "Create #{spec_name} database for current environment"
task spec_name => :load_config do
db_config = ActiveRecord::Base.configurations.configs_for(env_name: Rails.env, spec_name: spec_name)
@@ -42,7 +44,7 @@ db_namespace = namespace :db do
ActiveRecord::Tasks::DatabaseTasks.drop_all
end
- ActiveRecord::Tasks::DatabaseTasks.for_each do |spec_name|
+ ActiveRecord::Tasks::DatabaseTasks.for_each(databases) do |spec_name|
desc "Drop #{spec_name} database for current environment"
task spec_name => [:load_config, :check_protected_environments] do
db_config = ActiveRecord::Base.configurations.configs_for(env_name: Rails.env, spec_name: spec_name)
@@ -101,7 +103,7 @@ db_namespace = namespace :db do
end
namespace :migrate do
- ActiveRecord::Tasks::DatabaseTasks.for_each do |spec_name|
+ ActiveRecord::Tasks::DatabaseTasks.for_each(databases) do |spec_name|
desc "Migrate #{spec_name} database for current environment"
task spec_name => :load_config do
db_config = ActiveRecord::Base.configurations.configs_for(env_name: Rails.env, spec_name: spec_name)
@@ -142,7 +144,7 @@ db_namespace = namespace :db do
end
namespace :up do
- ActiveRecord::Tasks::DatabaseTasks.for_each do |spec_name|
+ ActiveRecord::Tasks::DatabaseTasks.for_each(databases) do |spec_name|
task spec_name => :load_config do
raise "VERSION is required" if !ENV["VERSION"] || ENV["VERSION"].empty?
@@ -176,7 +178,7 @@ db_namespace = namespace :db do
end
namespace :down do
- ActiveRecord::Tasks::DatabaseTasks.for_each do |spec_name|
+ ActiveRecord::Tasks::DatabaseTasks.for_each(databases) do |spec_name|
task spec_name => :load_config do
raise "VERSION is required" if !ENV["VERSION"] || ENV["VERSION"].empty?
@@ -203,7 +205,7 @@ db_namespace = namespace :db do
end
namespace :status do
- ActiveRecord::Tasks::DatabaseTasks.for_each do |spec_name|
+ ActiveRecord::Tasks::DatabaseTasks.for_each(databases) do |spec_name|
desc "Display status of migrations for #{spec_name} database"
task spec_name => :load_config do
db_config = ActiveRecord::Base.configurations.configs_for(env_name: Rails.env, spec_name: spec_name)
@@ -266,7 +268,7 @@ db_namespace = namespace :db do
end
namespace :abort_if_pending_migrations do
- ActiveRecord::Tasks::DatabaseTasks.for_each do |spec_name|
+ ActiveRecord::Tasks::DatabaseTasks.for_each(databases) do |spec_name|
# desc "Raises an error if there are pending migrations for #{spec_name} database"
task spec_name => :load_config do
db_config = ActiveRecord::Base.configurations.configs_for(env_name: Rails.env, spec_name: spec_name)
diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb
index aecc9350e8..a78bebf764 100644
--- a/activerecord/lib/active_record/tasks/database_tasks.rb
+++ b/activerecord/lib/active_record/tasks/database_tasks.rb
@@ -141,10 +141,19 @@ module ActiveRecord
end
end
- def for_each
+ def setup_initial_database_yaml
return {} unless defined?(Rails)
- databases = Rails.application.config.load_database_yaml
+ begin
+ Rails.application.config.load_database_yaml
+ rescue
+ $stderr.puts "Rails couldn't infer whether you are using multiple databases from your database.yml and can't generate the tasks for the non-primary databases. If you'd like to use this feature, please simplify your ERB."
+
+ {}
+ end
+ end
+
+ def for_each(databases)
database_configs = ActiveRecord::DatabaseConfigurations.new(databases).configs_for(env_name: Rails.env)
# if this is a single database application we don't want tasks for each primary database
diff --git a/railties/test/application/rake/dbs_test.rb b/railties/test/application/rake/dbs_test.rb
index e08cd09abd..79c521dbf6 100644
--- a/railties/test/application/rake/dbs_test.rb
+++ b/railties/test/application/rake/dbs_test.rb
@@ -40,6 +40,15 @@ module ApplicationTests
end
end
+ def db_create_with_warning(expected_database)
+ Dir.chdir(app_path) do
+ output = rails("db:create")
+ assert_match(/Rails couldn't infer whether you are using multiple databases/, output)
+ assert_match(/Created database/, output)
+ assert File.exist?(expected_database)
+ end
+ end
+
test "db:create and db:drop without database URL" do
require "#{app_path}/config/environment"
db_create_and_drop ActiveRecord::Base.configurations[Rails.env]["database"]
@@ -86,6 +95,25 @@ module ApplicationTests
db_create_and_drop("db/development.sqlite3", environment_loaded: false)
end
+ test "db:create and db:drop show warning but doesn't raise errors when loading YAML with alias ERB" do
+ app_file "config/database.yml", <<-YAML
+ sqlite: &sqlite
+ adapter: sqlite3
+ database: db/development.sqlite3
+
+ development:
+ <<: *<%= ENV["DB"] || "sqlite" %>
+ YAML
+
+ app_file "config/environments/development.rb", <<-RUBY
+ Rails.application.configure do
+ config.database = "db/development.sqlite3"
+ end
+ RUBY
+
+ db_create_with_warning("db/development.sqlite3")
+ end
+
test "db:create and db:drop don't raise errors when loading YAML containing conditional statements in ERB" do
app_file "config/database.yml", <<-YAML
development: