On Mon, 2013-02-18 at 16:27 +0100, mfoj...@redhat.com wrote:
> From: Michal Fojtik <mfoj...@redhat.com>
>
> We don't want to run any migrations automatically during
> Deltacloud server boot time, to give users chance to backup
> data, etc.
>
> This patch will provide a nice warning message if you start
> Deltacloud and you have some pending migrations to run.
>
> Signed-off-by: Michal fojtik <mfoj...@redhat.com>
> ---
> server/bin/deltacloud-db-upgrade | 18 +++++++++++-------
> server/lib/initializers/database_initialize.rb | 19 ++++++++++++++++++-
> 2 files changed, 29 insertions(+), 8 deletions(-)
>
> diff --git a/server/bin/deltacloud-db-upgrade
> b/server/bin/deltacloud-db-upgrade
> index 2698350..5b37bbb 100755
> --- a/server/bin/deltacloud-db-upgrade
> +++ b/server/bin/deltacloud-db-upgrade
> @@ -1,13 +1,17 @@
> #!/usr/bin/env ruby
>
> -ENV['API_VERBOSE'] = 'true'
> +require 'rubygems'
>
> -load File.join(File.dirname(__FILE__), '..', 'lib', 'db.rb')
> +require 'require_relative' if RUBY_VERSION < '1.9'
>
> -# Initialize the database
> -#
> -db = Deltacloud.initialize_database
> +require_relative './../lib/initializers/mock_initialize'
> +require_relative './../lib/initializers/database_initialize'
>
> -# Apply the migrations
> +# The DATABASE_UPGRADE constant is set to true if we have discovered
> +# pending migrations in DATABASE_MIGRATIONS_DIR.
> #
> -Sequel::Migrator.apply(db, File.join(File.dirname(__FILE__), '..', 'db',
> 'migrations'))
> +
> +if DATABASE_UPGRADE
> + puts "Upgrading database schema to the latest version..."
> + Sequel::Migrator.apply(DATABASE, DATABASE_MIGRATIONS_DIR)
> +end
>
> diff --git a/server/lib/initializers/database_initialize.rb
> b/server/lib/initializers/database_initialize.rb
> index 32badb2..ae9d247 100644
> --- a/server/lib/initializers/database_initialize.rb
> +++ b/server/lib/initializers/database_initialize.rb
> @@ -12,6 +12,9 @@ require_relative '../db'
> #
> Sequel::Model.plugin :validation_class_methods
>
> +# Enable Sequel migrations extension
> +Sequel.extension :migration
> +
> # For JRuby we need to different Sequel driver
> #
> sequel_driver = (RUBY_PLATFORM=='java') ? 'jdbc:sqlite:' : 'sqlite://'
> @@ -25,4 +28,18 @@ sequel_driver = (RUBY_PLATFORM=='java') ? 'jdbc:sqlite:' :
> 'sqlite://'
> DATABASE_LOCATION = ENV['DATABASE_LOCATION'] ||
> "#{sequel_driver}#{File.join(BASE_STORAGE_DIR, 'db.sqlite')}"
>
> -Deltacloud::initialize_database
> +DATABASE = Deltacloud::initialize_database
> +
> +# Detect if there are some pending migrations to run.
> +# We don't actually run migrations during server startup, just print
> +# a warning to console
> +#
> +
> +DATABASE_MIGRATIONS_DIR = File.join(File.dirname(__FILE__), '..', '..',
> 'db', 'migrations')
> +
> +unless Sequel::Migrator.is_current?(DATABASE, DATABASE_MIGRATIONS_DIR)
> + warn "WARNING: The database needs to be upgraded. Run:
> 'deltacloud-db-upgrade' command."
> + DATABASE_UPGRADE = true
> +else
> + DATABASE_UPGRADE = false
> +end
This strikes me as more clever than it needs to be:
* we should exit(1) if there are pending migrations - there's no
point in starting the server otherwise.
* can't deltacloud-db-upgrade just blindly run
Sequel::Migrator.run ? There shouldn't be any harm even if there
are no migrations to apply
* do we really need a separate deltacloud-db-upgrade script rather
than just the rake task ? I don't have an issue with requiring
that people have rake on their prod systems (what's good enough
for rails ought to be good enough for us ;)
But ACK anyway; would be nice though to address these.
David
