Skip to content

Commit

Permalink
Don't obscure errors during migrations (#577)
Browse files Browse the repository at this point in the history
  • Loading branch information
matthewmcgarvey authored Dec 28, 2020
1 parent 1e6f4ed commit f667249
Show file tree
Hide file tree
Showing 26 changed files with 93 additions and 112 deletions.
2 changes: 1 addition & 1 deletion spec/migrator/migration_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ end

describe Avram::Migrator::Migration::V1 do
it "executes statements in a transaction" do
expect_raises Exception, %(relation "table_does_not_exist" does not exist) do
expect_raises Avram::FailedMigration do
MigrationThatPartiallyWorks::V999.new.up
end

Expand Down
6 changes: 3 additions & 3 deletions spec/spec_helper.cr
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ backend = Log::IOBackend.new(STDERR)
backend.formatter = Dexter::JSONLogFormatter.proc
Log.builder.bind("avram.*", :error, backend)

Db::Create.new(quiet: true).call
Db::Migrate.new(quiet: true).call
Db::VerifyConnection.new(quiet: true).call
Db::Create.new(quiet: true).run_task
Db::Migrate.new(quiet: true).run_task
Db::VerifyConnection.new(quiet: true).run_task

Spec.before_each do
TestDatabase.truncate
Expand Down
2 changes: 1 addition & 1 deletion spec/tasks/db_create_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ describe Db::Create do
it "raises a connection error when unable to connect" do
Avram.temp_config(database_to_migrate: DatabaseWithIncorrectSettings) do
expect_raises(Exception, /It looks like Postgres is not running/) do
Db::Create.new(quiet: true).call
Db::Create.new(quiet: true).run_task
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/tasks/db_schema_dump_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ include CleanupHelper
describe Db::Schema::Dump do
it "generates a new sql dump file" do
with_cleanup do
Db::Schema::Dump.new("structure.sql").call
Db::Schema::Dump.new("structure.sql").run_task

filename = "structure.sql"
File.exists?(filename).should eq true
Expand Down
10 changes: 5 additions & 5 deletions spec/tasks/db_schema_restore_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,22 @@ SQL_DUMP_FILE = "spec/support/files/sample_backup.sql"
describe Db::Schema::Restore do
it "raises an error when no import file is supplied" do
expect_raises(Exception, "A path to the import SQL file must be provided") do
Db::Schema::Restore.new.call
Db::Schema::Restore.new.run_task
end
end

it "raises an error when unable to find the import file" do
expect_raises(Exception, "Unable to locate the restore file: missing_file.sql") do
Db::Schema::Restore.new("missing_file.sql").call
Db::Schema::Restore.new("missing_file.sql").run_task
end
end

it "restores from the sample_backup file" do
Avram.temp_config(database_to_migrate: SampleBackupDatabase) do
Db::Drop.new.call
Db::Create.new(quiet: true).call
Db::Drop.new.run_task
Db::Create.new(quiet: true).run_task

Db::Schema::Restore.new(SQL_DUMP_FILE).call
Db::Schema::Restore.new(SQL_DUMP_FILE).run_task

SampleBackupDatabase.run do |db|
value = db.scalar("SELECT COUNT(*) FROM sample_records").as(Int64)
Expand Down
2 changes: 1 addition & 1 deletion spec/tasks/db_verify_connection_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ describe Db::VerifyConnection do
creds = Avram::Credentials.parse("postgres://eat@joes/crab_shack")
TestDatabase.temp_config(credentials: creds) do
expect_raises Exception, /Unable to connect to Postgres for database 'TestDatabase'/ do
Db::VerifyConnection.new.call
Db::VerifyConnection.new.run_task
end
end
end
Expand Down
17 changes: 17 additions & 0 deletions src/avram/errors.cr
Original file line number Diff line number Diff line change
Expand Up @@ -146,4 +146,21 @@ module Avram
# Used when `Avram::Operation` fails.
class FailedOperation < AvramError
end

class FailedMigration < AvramError
def initialize(@migration : String, statements : Array(String), cause : Exception)
super(message(statements), cause)
end

private def message(statements : Array(String))
<<-ERROR
Error occurred while performing a migration.
Migration: #{@migration}
Statements:
#{statements.join("\n")}
ERROR
end
end
end
10 changes: 1 addition & 9 deletions src/avram/migrator/migration.cr
Original file line number Diff line number Diff line change
Expand Up @@ -97,15 +97,7 @@ abstract class Avram::Migrator::Migration::V1
end
end
rescue e : PQ::PQError
raise <<-ERROR
There was a problem running this statement:
#{statements.join("\n")}
Problem:
#{e.message}
ERROR
raise FailedMigration.new(migration: self.class.name, statements: statements, cause: e)
end

def reset_prepared_statements
Expand Down
7 changes: 3 additions & 4 deletions src/avram/migrator/migrator.cr
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
module Avram::Migrator
def self.run
yield
rescue e : PQ::PQError
raise e.message.colorize(:red).to_s
rescue e : Exception
raise e.message.to_s
rescue e
puts e.inspect_with_backtrace
exit 1
end
end
4 changes: 1 addition & 3 deletions src/avram/migrator/runner.cr
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ class Avram::Migrator::Runner
end

def self.dump_db(dump_to : String = "db/structure.sql", quiet : Bool = false)
Db::VerifyConnection.new(quiet: true).call
Db::VerifyConnection.new(quiet: true).run_task
run "pg_dump -s #{cmd_args} > #{dump_to}"
unless quiet
puts "Done dumping #{db_name.colorize(:green)}"
Expand Down Expand Up @@ -189,7 +189,5 @@ class Avram::Migrator::Runner
end
rescue e : DB::ConnectionRefused
raise "Unable to connect to the database. Please check your configuration.".colorize(:red).to_s
rescue e : Exception
raise "Unexpected error while running migrations: #{e.message}".colorize(:red).to_s
end
end
9 changes: 9 additions & 0 deletions src/avram/tasks/db/base_task.cr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
abstract class BaseTask < LuckyCli::Task
abstract def run_task

def call
Avram::Migrator.run do
run_task
end
end
end
8 changes: 3 additions & 5 deletions src/avram/tasks/db/create.cr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class Db::Create < LuckyCli::Task
class Db::Create < BaseTask
alias Migrator = Avram::Migrator
summary "Create the database"

Expand All @@ -19,9 +19,7 @@ class Db::Create < LuckyCli::Task
TEXT
end

def call
Migrator.run do
Migrator::Runner.create_db(@quiet)
end
def run_task
Migrator::Runner.create_db(@quiet)
end
end
10 changes: 4 additions & 6 deletions src/avram/tasks/db/drop.cr
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
require "colorize"

class Db::Drop < LuckyCli::Task
class Db::Drop < BaseTask
summary "Drop the database"

def call
Avram::Migrator.run do
Avram::Migrator::Runner.drop_db
puts "Done dropping #{Avram::Migrator::Runner.db_name.colorize(:green)}"
end
def run_task
Avram::Migrator::Runner.drop_db
puts "Done dropping #{Avram::Migrator::Runner.db_name.colorize(:green)}"
end

def help_message
Expand Down
8 changes: 3 additions & 5 deletions src/avram/tasks/db/migrate.cr
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require "colorize"

class Db::Migrate < LuckyCli::Task
class Db::Migrate < BaseTask
summary "Run any pending migrations"

def initialize(@quiet : Bool = false)
Expand All @@ -18,9 +18,7 @@ class Db::Migrate < LuckyCli::Task
TEXT
end

def call
Avram::Migrator.run do
Avram::Migrator::Runner.new(@quiet).run_pending_migrations
end
def run_task
Avram::Migrator::Runner.new(@quiet).run_pending_migrations
end
end
8 changes: 3 additions & 5 deletions src/avram/tasks/db/migrate_one.cr
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require "colorize"

class Db::Migrate::One < LuckyCli::Task
class Db::Migrate::One < BaseTask
summary "Run just the next pending migration"

def help_message
Expand All @@ -14,9 +14,7 @@ class Db::Migrate::One < LuckyCli::Task
TEXT
end

def call
Avram::Migrator.run do
Avram::Migrator::Runner.new.run_next_migration
end
def run_task
Avram::Migrator::Runner.new.run_next_migration
end
end
4 changes: 2 additions & 2 deletions src/avram/tasks/db/migrations_status.cr
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
require "shell-table"

class Db::Migrations::Status < LuckyCli::Task
class Db::Migrations::Status < BaseTask
summary "Print the current status of migrations"

def call
def run_task
if migrations.none?
puts "There are no migrations.".colorize(:green)
else
Expand Down
8 changes: 4 additions & 4 deletions src/avram/tasks/db/redo.cr
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require "colorize"

class Db::Redo < LuckyCli::Task
class Db::Redo < BaseTask
summary "Rollback and run just the last migration"

def help_message
Expand All @@ -14,8 +14,8 @@ class Db::Redo < LuckyCli::Task
TEXT
end

def call
Db::Rollback.new.call
Db::Migrate.new.call
def run_task
Db::Rollback.new.run_task
Db::Migrate.new.run_task
end
end
10 changes: 5 additions & 5 deletions src/avram/tasks/db/reset.cr
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require "colorize"

class Db::Reset < LuckyCli::Task
class Db::Reset < BaseTask
summary "Drop, recreate, and run migrations."

def help_message
Expand All @@ -18,9 +18,9 @@ class Db::Reset < LuckyCli::Task
TEXT
end

def call
Db::Drop.new.call
Db::Create.new.call
Db::Migrate.new.call
def run_task
Db::Drop.new.run_task
Db::Create.new.run_task
Db::Migrate.new.run_task
end
end
8 changes: 3 additions & 5 deletions src/avram/tasks/db/rollback.cr
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require "colorize"

class Db::Rollback < LuckyCli::Task
class Db::Rollback < BaseTask
summary "Rollback the last migration"

def help_message
Expand All @@ -14,9 +14,7 @@ class Db::Rollback < LuckyCli::Task
TEXT
end

def call
Avram::Migrator.run do
Avram::Migrator::Runner.new.rollback_one
end
def run_task
Avram::Migrator::Runner.new.rollback_one
end
end
10 changes: 4 additions & 6 deletions src/avram/tasks/db/rollback_all.cr
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require "colorize"

class Db::RollbackAll < LuckyCli::Task
class Db::RollbackAll < BaseTask
summary "Rollback all migrations"

def help_message
Expand All @@ -16,10 +16,8 @@ class Db::RollbackAll < LuckyCli::Task
TEXT
end

def call
Avram::Migrator.run do
Avram::Migrator::Runner.new.rollback_all
puts "Done rolling back all migrations".colorize(:green)
end
def run_task
Avram::Migrator::Runner.new.rollback_all
puts "Done rolling back all migrations".colorize(:green)
end
end
16 changes: 7 additions & 9 deletions src/avram/tasks/db/rollback_to.cr
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require "colorize"

class Db::RollbackTo < LuckyCli::Task
class Db::RollbackTo < BaseTask
summary "Rollback to a specific migration"

def help_message
Expand All @@ -16,14 +16,12 @@ class Db::RollbackTo < LuckyCli::Task
TEXT
end

def call
Avram::Migrator.run do
version = ARGV.first?
if version && version.to_i64?
Avram::Migrator::Runner.new.rollback_to(version.to_i64)
else
raise "Migration version is required. Example: lucky db.rollback_to 20180802180356".colorize(:red).to_s
end
def run_task
version = ARGV.first?
if version && version.to_i64?
Avram::Migrator::Runner.new.rollback_to(version.to_i64)
else
raise "Migration version is required. Example: lucky db.rollback_to 20180802180356".colorize(:red).to_s
end
end
end
8 changes: 3 additions & 5 deletions src/avram/tasks/db/schema_dump.cr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class Db::Schema::Dump < LuckyCli::Task
class Db::Schema::Dump < BaseTask
summary "Export database schema to a sql file"

def initialize(@dump_to : String? = nil, @quiet : Bool = false)
Expand All @@ -18,10 +18,8 @@ class Db::Schema::Dump < LuckyCli::Task
TEXT
end

def call
def run_task
dump_to = @dump_to || ARGV.first? || raise "Must pass a file path to dump the db structure to"
Avram::Migrator.run do
Avram::Migrator::Runner.dump_db(dump_to, @quiet)
end
Avram::Migrator::Runner.dump_db(dump_to, @quiet)
end
end
8 changes: 3 additions & 5 deletions src/avram/tasks/db/schema_restore.cr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class Db::Schema::Restore < LuckyCli::Task
class Db::Schema::Restore < BaseTask
summary "Restore database from a sql dump file"

def initialize(@import_file_path : String? = nil, @quiet : Bool = false)
Expand All @@ -17,10 +17,8 @@ class Db::Schema::Restore < LuckyCli::Task
TEXT
end

def call
def run_task
import_file_path = @import_file_path || ARGV.first? || raise "A path to the import SQL file must be provided"
Avram::Migrator.run do
Avram::Migrator::Runner.restore_db(import_file_path.as(String), @quiet)
end
Avram::Migrator::Runner.restore_db(import_file_path.as(String), @quiet)
end
end
Loading

0 comments on commit f667249

Please sign in to comment.