deltacloud-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Lutterkort <lut...@redhat.com>
Subject Re: [PATCH core 7/7] CIMI: Print warning for pending migrations
Date Tue, 19 Feb 2013 01:18:15 GMT
On Mon, 2013-02-18 at 16:27 +0100, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mfojtik@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 <mfojtik@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



Mime
View raw message