Permalink
Browse files

Explicitly exit with status "1" for create and drop failures

* A non-zero exit status allows subsequent shell commands to be chained
  together such as: `rake db:reset test:prepare && rspec && cap deploy`
  (if you're feeling brave :)
* Any exceptions raised during the `create` and `drop` tasks are caught
  in order to print a "pretty" message to the user. Unfortunately doing
  so prevents rake from aborting with a non-zero exit status to the shell.
* Therefore we re-raise the exceptions after the "pretty" message and
  re-catch them in the task.
* From the task we explicitly exit with a non-zero status. This method
  was chosen (rather than just letting rake fail from the exception) so
  that the backtrace is suppressed and the output to stderr is
  unchanged.
* Update activerecord CHANGELOG
  • Loading branch information...
iamvery committed Oct 14, 2013
1 parent a11ddbe commit 22f80ae57b26907f662b7fd50a7270a6381e527e
@@ -1,3 +1,7 @@
+* Exit with non-zero status for failed database rake tasks.
+
+ *Jay Hayes*
+
* Generate subquery for `Relation` if it passed as array condition for `where`
method.
@@ -8,31 +8,47 @@ db_namespace = namespace :db do
namespace :create do
task :all => :load_config do
- ActiveRecord::Tasks::DatabaseTasks.create_all
+ begin
+ ActiveRecord::Tasks::DatabaseTasks.create_all
+ rescue
+ exit(1)
+ end
end
end
desc 'Create the database from DATABASE_URL or config/database.yml for the current Rails.env (use db:create:all to create all databases in the config)'
task :create => [:load_config] do
- if ENV['DATABASE_URL']
- ActiveRecord::Tasks::DatabaseTasks.create_database_url
- else
- ActiveRecord::Tasks::DatabaseTasks.create_current
+ begin
+ if ENV['DATABASE_URL']
+ ActiveRecord::Tasks::DatabaseTasks.create_database_url
+ else
+ ActiveRecord::Tasks::DatabaseTasks.create_current
+ end
+ rescue
+ exit(1)
end
end
namespace :drop do
task :all => :load_config do
- ActiveRecord::Tasks::DatabaseTasks.drop_all
+ begin
+ ActiveRecord::Tasks::DatabaseTasks.drop_all
+ rescue
+ exit(1)
+ end
end
end
desc 'Drops the database using DATABASE_URL or the current Rails.env (use db:drop:all to drop all databases)'
task :drop => [:load_config] do
- if ENV['DATABASE_URL']
- ActiveRecord::Tasks::DatabaseTasks.drop_database_url
- else
- ActiveRecord::Tasks::DatabaseTasks.drop_current
+ begin
+ if ENV['DATABASE_URL']
+ ActiveRecord::Tasks::DatabaseTasks.drop_database_url
+ else
+ ActiveRecord::Tasks::DatabaseTasks.drop_current
+ end
+ rescue
+ exit(1)
end
end
@@ -69,9 +69,11 @@ def create(*arguments)
class_for_adapter(configuration['adapter']).new(*arguments).create
rescue DatabaseAlreadyExists
$stderr.puts "#{configuration['database']} already exists"
+ raise
rescue Exception => error
$stderr.puts error, *(error.backtrace)
$stderr.puts "Couldn't create database for #{configuration.inspect}"
+ raise
end
def create_all
@@ -95,6 +97,7 @@ def drop(*arguments)
rescue Exception => error
$stderr.puts error, *(error.backtrace)
$stderr.puts "Couldn't drop #{configuration['database']}"
+ raise
end
def drop_all
@@ -61,7 +61,9 @@ def test_create_when_database_exists_outputs_info_to_stderr
ActiveRecord::StatementInvalid.new("Can't create database 'dev'; database exists:")
)
- ActiveRecord::Tasks::DatabaseTasks.create @configuration
+ assert_raises(ActiveRecord::Tasks::DatabaseAlreadyExists) do
+ ActiveRecord::Tasks::DatabaseTasks.create @configuration
+ end
end
end
@@ -59,7 +59,9 @@ def test_db_create_with_error_prints_message
$stderr.expects(:puts).
with("Couldn't create database for #{@configuration.inspect}")
- ActiveRecord::Tasks::DatabaseTasks.create @configuration
+ assert_raises(Exception) do
+ ActiveRecord::Tasks::DatabaseTasks.create @configuration
+ end
end
def test_create_when_database_exists_outputs_info_to_stderr
@@ -69,7 +71,9 @@ def test_create_when_database_exists_outputs_info_to_stderr
ActiveRecord::StatementInvalid.new('database "my-app-db" already exists')
)
- ActiveRecord::Tasks::DatabaseTasks.create @configuration
+ assert_raises(ActiveRecord::Tasks::DatabaseAlreadyExists) do
+ ActiveRecord::Tasks::DatabaseTasks.create @configuration
+ end
end
end
@@ -27,7 +27,9 @@ def test_db_create_when_file_exists
$stderr.expects(:puts).with("#{@database} already exists")
- ActiveRecord::Tasks::DatabaseTasks.create @configuration, '/rails/root'
+ assert_raises(ActiveRecord::Tasks::DatabaseAlreadyExists) do
+ ActiveRecord::Tasks::DatabaseTasks.create @configuration, '/rails/root'
+ end
end
def test_db_create_with_file_does_nothing
@@ -36,7 +38,9 @@ def test_db_create_with_file_does_nothing
ActiveRecord::Base.expects(:establish_connection).never
- ActiveRecord::Tasks::DatabaseTasks.create @configuration, '/rails/root'
+ assert_raises(ActiveRecord::Tasks::DatabaseAlreadyExists) do
+ ActiveRecord::Tasks::DatabaseTasks.create @configuration, '/rails/root'
+ end
end
def test_db_create_establishes_a_connection
@@ -52,7 +56,9 @@ def test_db_create_with_error_prints_message
$stderr.expects(:puts).
with("Couldn't create database for #{@configuration.inspect}")
- ActiveRecord::Tasks::DatabaseTasks.create @configuration, '/rails/root'
+ assert_raises(Exception) do
+ ActiveRecord::Tasks::DatabaseTasks.create @configuration, '/rails/root'
+ end
end
end

4 comments on commit 22f80ae

Owner

rafaelfranca replied Nov 19, 2013

@iamvery this changed the behavior of db:setup task that is not overwrite an existing database anymore and db:drop that is showing stacktrace. Could you investigate and submit again? I'm reverting this now because it broke the @rubys suite.

Contributor

rubys replied Nov 19, 2013

db:drop when database doesn't exist causing a stack trace is unrelated. In other words, it continues to do so even after this commit is reverted.

Contributor

iamvery replied Nov 19, 2013

Bummer!

Just to make sure I fully understand. The second thing you mentioned (i.e. showing a stacktrace for db:drop) is not related to this changeset and does not need to be addressed by this issue.

The problem that should be addressed is restoring the previous behavior of the db:setup task. Namely to ensure that subsequent calls to db:setup should overwrite existing databases with each call.

Am I understanding that correctly @rubys and @rafaelfranca?

Contributor

iamvery replied Nov 19, 2013

New PR at #12956

Please sign in to comment.