Add Lotus::Model::Migration #144

runlevel5 · 2015-01-25T11:52:33Z

[Fixes #115]

This adds the basic migration feature for Lotus.

Please be noted that this PR does not cover load/dump schema.rb, I am going to have a separate PR for that.

TODO

Migration class
Timestamp Migrator (Noted: this is totally different to Sequel::TimestampMigrator)
Schema.rb dump
Write CLI task 'lotus generate migration' to generate new migration
Write CLI task 'lotus db migrate' to migrate
Write CLI task 'lotus db rollback' to rollback
Write CLI task 'lotus db apply' to merge all changes to schema.rb and remove migration files

Migration

It purely base on Sequel::Migration and thus share the same interface as Sequel::Migration:

class MyMigration < Lotus::Model::Migration
  def up
  end

  def down
  end
end

Migration files are located inside LOTUS_ROOT/db/migrations folder by default. However you can configure it with Lotus::Model::Configuration#migration_directory

Migrator

To migrate, we use Migrator:

require 'lotus/model'
require 'lotus/model/migrator'

#adapter configuration is copied from the framework configuration
# so make sure you have to configure framework first or else
# you will get error!    
Lotus::Model.configure do
  migration_directory 'my_apps/db/migrations'
  adapter type: :sql, uri: 'sqlite3://localhost/database'
end

Lotus::Model::Migrator.new.run

To migrate from a directory

Lotus::Model::Migrator.new.migrate

To rollback to previous migration

Lotus::Model::Migrator.new.rollback

or migration 4 steps from the last version

Lotus::Model::Migrator.new.rollback(step: 4)

Furthermore, I also introduce the ability to set logger for the framework. Please see Lotus::Model::Configuration#logging. The migrator will use the framework logger implicitly.

Schema Modification API

create_table

Use to create new table. Syntax to add new column is below:

create_table(:songs) do
  column :id, primary_key: true
  column :title, :string, null: false
  column :artist, :string, 'VARCHAR(15)'
end

AlfonsoUceda · 2015-01-26T14:45:20Z

lib/lotus/model/migration.rb

+require 'sequel'
+require 'sequel/extensions/migration'
+
+# Allow users to easily group schema changes # and migrate the


changes # and
to
changes and

AlfonsoUceda · 2015-02-10T08:40:56Z

lib/lotus/model/migration.rb

+
+module Lotus
+  module Model
+    # Allow users to easily group schema changes # and migrate the


/s/changes # and/changes and/

janlelis · 2015-02-25T19:56:01Z

Hi everybody,
I want to suggest a small detail about migrations: Some years ago I wrote a mini library for Rails migrations that would do no more than replacing

class SomeMigration < ActiveRecord::Migration
  # ...

with

migration do
  # ...

which generated the class name automatically, based on the filename. The motivation behind this was that when I wanted to create a new migration (without using Rails' generator), I would always need to come up with a new, unique class name, that I never needed again. Having a short migration do solved this problem for me.

Unfortunately, it introduces magic, but something similar/better could be done in a not-so-magic manner:

Lotus::Model::Migration.create "description" do
  # ... (the description being also generated, but it is an optional parameter)

I propose this little API change, because:

It frees you from making up new constant names
Instead, it gives you the possibility to describe what you want to do in a free format (rspec describe style)
The description string can be used to generate an unique migration class (random one, if none given)

I am happy to write the code for this, if you like the idea.

runlevel5 · 2015-02-26T00:51:59Z

@janlelis Thanks much for your detailed feedback.

I have been looking into the DSL solution from the very start of this feature. In fact, doing DSL is very straight-forward because Sequel already had DSL for migration.

After weighting pros/cons, I decided to stay away from DSL to avoid unnecessary complexity.

I acknowledge your pain point regarding unique class constant however I don't think it would be a huge hassle because Lotus would provide migration generator. Besides, your suggestion on class name inflection from filename is not going to work out of the box though, for schema_migration table stores the full _filepath into filename column and should users have migrated (not rollback yet) then rename the filename would create extra record in schema_migration table.

Having said all of the above, I am open for feedbacks from other uses, if this is truly a pain point, I am more than happy to make changes.

mwallasch · 2015-02-27T13:38:50Z

@joneslee85 Great work. One small question: is there a good reason to keep the current_version method private. Sometimes it would be helpful to have the possibility for a quick check for the current migrated version.

runlevel5 · 2015-02-27T13:42:13Z

@mwallasch there is no particular reason. I did not think of your scenario. Thanks for the feedback. I'll make change.

jodosha · 2015-02-27T13:59:18Z

@joneslee85 In the description and in the inline docs you say that the default logger is Lotus::Logger, which is included by lotusrb, but in the end the code uses Ruby's stdlib ::Logger. This because it makes it hard to reuse that component from our main gem. Do you think we should extract it into lotus-utils, so it can be available everywhere?

jodosha · 2015-02-27T14:02:45Z

lib/lotus/model/migration.rb

+    #   end
+    #
+    #   # to migrate
+    #   MyMigration.apply(DB, :up)


@joneslee85 This comment puzzles me. 😺 DB derives from Sequel, but what's the meaning here in Lotus::Model?

Yes, the Migration class is low-level class which has access to apply method and that method takes in a sequel connection. I think I am going to document that for whom who care about this low level detail or should I remove it?

Well, we need to document anyway how to apply a migration. Please keep the documentation, but with a Lotus terminology. 😄

runlevel5 · 2015-02-27T14:03:53Z

@jodosha I forget to update the description. I don't think it is necessary to extract it to lotus/utils. Btw, I have removed setting logger to ::Logger, the framework should set this explicitly. By default, logger is nil.

jodosha · 2015-02-27T14:11:42Z

lib/lotus/model/migration.rb

+        end
+      end
+
+      MIGRATION_DIRECTORY = "#{Dir.pwd}/db/migrations".freeze


Should we split the composition of this directory? If feel uneasy to leave a frozen constant with the current value that can possibly change. Eg. if someone uses Dir.chdir. Can we interpolate at the run time?

Example:

MIGRATION_DIRECTORY = 'db/migrations'.freeze

And then in the methods that use it:

directory = "#{ Dir.pwd }/#{ MIGRATION_DIRECTORY }"

We can have a private method to do this tedious interpolation over and over.

huydx · 2015-03-01T10:58:28Z

lib/lotus/model/configuration.rb

+        else
+          @logger ||= logger
+        end
+      end


what is the difference bw logging and logger?, according to the sample, it's seem to be the same thing

I agree, I'd renamed to def logger(value = nil) IMO

they are pretty much the same. the logging method acts as accessor DSL (though I hate it), I don't mind doing it the Ruby way, that is:

logger = Something

at all. But I don't want to break the current pattern without running it through with everyone.

AlfonsoUceda · 2015-03-01T11:20:34Z

lib/lotus/model/migrator.rb

+        end
+      end
+
+      SUPPORTED_ADAPTERS = [:sql].freeze


document this constant

AlfonsoUceda · 2015-03-04T10:17:59Z

👍 ❤️

jodosha · 2015-03-04T10:32:47Z

lib/lotus/model/configuration.rb

+      # @since 0.3.0
+      # @api private
+      def _default_migration_directory
+        "#{Dir.pwd}/db/migrations"


What about Pathname.pwd.join('db', 'migrations')? This will make the code OS independent.

jodosha · 2015-04-24T13:18:10Z

lib/lotus/model/adapters/sql/schema_modifier.rb

+        # @api private
+        class SchemaModifier
+          def initialize(adapter, logger = nil)
+            @connection = adapter.instance_variable_get(:@connection)


@joneslee85 Shouldn't this be a public method marked as private API? So we can avoid instance_variable_get and have adapter.connection.

deepj · 2015-05-28T21:45:08Z

I may be out but I'd like to suggest to split "schema" and "data" migrations. In Rails days most of developers putting "data" migration into "schema" migration. I often see something like:

Model.all.each do |model|
  model.update_attributes(some_attribute: 'something')
end

After many years I feel this is very bad approach leading to pain and very long time taking migrations.

I don't have a concrete idea how it would work. It's just a suggestion.

runlevel5 · 2015-05-29T01:00:42Z

@deepj yes, thanks for the input. My intent is to only support schema migration, for data migration I'd let user do it with rake task

deepj · 2015-05-29T09:35:12Z

OK, than isn't is better to name the migration class as Lotus::Model::SchemaMigration rather than Lotus::Model::Migration? I know it's a bit longer. I believe it'll be more obvious for what purposes is meant.

runlevel5 · 2015-05-30T07:51:25Z

@deepj I don't think it is necessary. Why? Because we follow the same sequel's class naming and so far it has been used just for schema changes only.

jodosha · 2015-06-17T07:58:36Z

Closed in favor of #196

runlevel5 added the wip label Jan 25, 2015

AlfonsoUceda reviewed Jan 26, 2015
View reviewed changes

This was referenced Feb 9, 2015

Lotus DB migration migrate runner hanami/hanami#137

Closed

Lotus DB migration rollback runner hanami/hanami#138

Closed

AlfonsoUceda reviewed Feb 10, 2015
View reviewed changes

runlevel5 removed the wip label Feb 27, 2015

jodosha reviewed Feb 27, 2015
View reviewed changes

runlevel5 self-assigned this Mar 1, 2015

runlevel5 added wip and removed wip labels Mar 1, 2015

huydx reviewed Mar 1, 2015
View reviewed changes

AlfonsoUceda reviewed Mar 1, 2015
View reviewed changes

lib/lotus/model/migrator.rb

end

end

SUPPORTED_ADAPTERS = [:sql].freeze

Copy link

Contributor

AlfonsoUceda Mar 1, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

document this constant

jodosha reviewed Mar 4, 2015
View reviewed changes

runlevel5 mentioned this pull request Apr 19, 2015

Add migration mechanism #115

Closed

runlevel5 added the wip label Apr 21, 2015

runlevel5 added this to the v0.4.0 milestone Apr 21, 2015

jodosha reviewed Apr 24, 2015
View reviewed changes

Trung Lê added 3 commits May 26, 2015 23:11

Add Lotus::Model::Migration

3b3671a

s/migration_dir/migrations_dir

f256101

Introduce SqlAdapter::Connection

f489cdc

jodosha closed this Jun 17, 2015

jodosha removed this from the v0.4.0 milestone Jun 17, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Lotus::Model::Migration #144

Add Lotus::Model::Migration #144

runlevel5 commented Jan 25, 2015

AlfonsoUceda Jan 26, 2015

AlfonsoUceda Feb 10, 2015

janlelis commented Feb 25, 2015

runlevel5 commented Feb 26, 2015

mwallasch commented Feb 27, 2015

runlevel5 commented Feb 27, 2015

jodosha commented Feb 27, 2015

jodosha Feb 27, 2015

runlevel5 Feb 27, 2015

jodosha Feb 27, 2015

runlevel5 commented Feb 27, 2015

jodosha Feb 27, 2015

huydx Mar 1, 2015

AlfonsoUceda Mar 1, 2015

runlevel5 Mar 1, 2015

AlfonsoUceda Mar 1, 2015

AlfonsoUceda commented Mar 4, 2015

jodosha Mar 4, 2015

jodosha Apr 24, 2015

deepj commented May 28, 2015

runlevel5 commented May 29, 2015

deepj commented May 29, 2015

runlevel5 commented May 30, 2015

jodosha commented Jun 17, 2015

Add Lotus::Model::Migration #144

Add Lotus::Model::Migration #144

Conversation

runlevel5 commented Jan 25, 2015

TODO

Migration

Migrator

Schema Modification API

create_table

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janlelis commented Feb 25, 2015

runlevel5 commented Feb 26, 2015

mwallasch commented Feb 27, 2015

runlevel5 commented Feb 27, 2015

jodosha commented Feb 27, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

runlevel5 commented Feb 27, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlfonsoUceda commented Mar 4, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deepj commented May 28, 2015

runlevel5 commented May 29, 2015

deepj commented May 29, 2015

runlevel5 commented May 30, 2015

jodosha commented Jun 17, 2015