data-migrate icon indicating copy to clipboard operation
data-migrate copied to clipboard

Usage with Rails Engines

Open tleish opened this issue 4 years ago • 12 comments

Can some point me to best practice of using this gem with Rails engines, where each Rails Engine stores their own data migrations as they do with schema migrations? (see: https://blog.pivotal.io/labs/labs/leave-your-migrations-in-your-rails-engines)

tleish avatar Dec 20 '20 00:12 tleish

Hm, good question. I never really thought about it in terms of engines. The gem should really be used in whatever place has it's own database. So theoretically, it should work in an engine. Practically, I haven't tried it. Does it actually work?

ilyakatz avatar Dec 22 '20 01:12 ilyakatz

I added the following into each engine.rb

      # Add seeds.rb to migrations
      data_migrations_path = Array(::DataMigrate.config.data_migrations_path)
      data_migrations_path << File.expand_path('db/data', config.root)
      ::DataMigrate.config.data_migrations_path = data_migrations_path

rake data:migrate picks up tall the migrations, runs the data migrations and records them to the data_migrations table. However, other commands do not work. For example, I get an error when running rake data:schema:load, which is why I wondered if there was a better approach to configuring data migrations with engines.

Was there initial work that never finished to support multiple paths? For example, you can find the following method

# data_migrate/data_migrator_five.rb
def self.migrations_paths
    [DataMigrate.config.data_migrations_path]
end

tleish avatar Dec 22 '20 06:12 tleish

I'm trying this as well, using the same technique as OP regarding migrations. I agree with @tleish that support for multiple paths will be needed, which will allow for a flow similar to what we do for migrations. The main app will need to load data_migrate, though, to get the rake tasks in the right place. Then if individual engines can add their data migrations to a path much like we do for schema migrations, I think everything should work nicely.

@ilyakatz I'd be willing to do this work if it's interests you and you're willing to review it.

kyrofa avatar Sep 02 '21 21:09 kyrofa

Hi @kyrofa @ilyakatz and @tleish !

We're also looking to allow for many values of DataMigrate.config.data_migrations_path. Ideally, this could accept a glob, but it would be fine if it just accepted an array of file paths (and we can expand out our own glob). Is something you'd consider accepting a PR for?

alexevanczuk avatar Aug 18 '22 17:08 alexevanczuk

Oh sorry, looks like I missed the previous comment. And yes, I'm happy to take in a PR, I haven't been actively maintaining this (or working in Rails for that matter), so happy to merge incoming PRs

ilyakatz avatar Aug 18 '22 17:08 ilyakatz

Thanks @ilyakatz ... ya know, as it turns out, we're already able to pass multiple paths to the configuration! The value ends up being massed through to the MigrationContext which calls Array(...) with the value and then globs out that array, so it already works the way we need! Thanks for your response.

alexevanczuk avatar Aug 19 '22 14:08 alexevanczuk

Oh great, maybe one of the Rails upgrades made it possible. Do you mind updating Readme to indicate this, it may help other folks!

ilyakatz avatar Aug 19 '22 16:08 ilyakatz

Can do!

We had some odd failures in CI related to this change so I'm just going to dig in and confirm it's related to our setup! Then I'll add to the README once I'm more confident other clients will be able to use the same approach 🙏

alexevanczuk avatar Aug 20 '22 13:08 alexevanczuk

@AlexEvanczuk any updates on that? We are really looking forward to that 😄

gustavodiel avatar Aug 31 '22 19:08 gustavodiel

@gustavodiel So we were able to get migrations to run locally with this:

DataMigrate.configure do |config|
   default_path = DataMigrate::Config.new.data_migrations_path
   package_packs = Stimpack::Packs.all.map { |pack| pack.path.relative_path_from(Dir.pwd).join('db/data/') }.map(&:to_s)
   config.data_migrations_path = [default_path, *package_packs]
 end

However, on CI we had this error:

Errno::ENAMETOOLONG: File name too long @ dir_initialize - 
packs/a/db/datapacksb/db/data/packsc/db/data[and so on]
--
  | /var/www/vendor/bundle/ruby/2.7.0/gems/data_migrate-8.0.0/lib/data_migrate/data_schema.rb:34:in `open'

I think for our purposes, we are actually going to keep migrations in /db/data and stop this investigation for now since we don't get a ton of benefit from distributing migrations in subfolders.

alexevanczuk avatar Sep 07 '22 15:09 alexevanczuk

@alexevanczuk That seems to be an issue with Stimpack. What we did was for each engines, in the lib/engine.rb file we added:

    initializer :engine do |app|
      ::DataMigrate.configure do |data_migrate|
        default_path = ::DataMigrate::Config.new.data_migrations_path
        data_migrate.data_migrations_path = [default_path, root.join('db', 'data')]
      end
    end

and worked flawlessly 😄

gustavodiel avatar Sep 12 '22 18:09 gustavodiel

@gustavodiel so we've tried doing exactly what you did, and we get two problems

  1. The file ends up going to db/data/Users/spence1115/git/.../engines/some_engine/db/data/20230301021917_populate_thing.rb
  2. It seems to always only pick up the file name of the last engine added

It also only read from one engine. We changed to

::DataMigrate.configure do |data_migrate|
        new_path = root.join("db", "data")
        current_path = ::DataMigrate.config.data_migrations_path
        data_migrate.data_migrations_path = if current_path.instance_of?(Array)
          current_path.concat([new_path])
        else
          [current_path, new_path]
        end
      end

in each engine, which meant it picked up and ran migrations for each engine, but still doesn't work for creating the migration in the first place :/

Spence1115 avatar Mar 01 '23 02:03 Spence1115