only migrate specified shards.

[kevinjcoleman]

No description provided.

[thiagopradi]

Since this PR is part of the Rails 5.2 migration, I'm merging it.

[PhilCoggins]

Doesn't this changeset make the assumption that all migrations should be run across any shards returned from migrations.map(&:shards).flatten.map(&:to_s)? What if I only want to run migrations on specific shards, as outlined in the readme?

EDIT: Gonna ping @kevinjcoleman @thiagopradi to try to get this sorted out, I now have migrations being run on a shard with a different schema than my primary shard.

[kevinjcoleman]

@PhilCoggins you're correct i think. Before it was sending it all the shards for a connection which was all shards anyway. I've been meaning to look into this since I've noticed the same thing with our migrations. My current fix has been to just check the current shard for that migration and skip if I don't want it to be run there. IE:

def change
  return unless User.current_shard == "master"
  # do regular migration stuff just on master DB
end 

[PhilCoggins]

Thanks for the response, I fixed with a quick monkey patch below, but have no idea what impact it may have on other systems. I think this needs to be prioritized and fixed rather quickly to prevent people messing up their schemas (hopefully they're testing!).

module Octopus
  module Migrator
    def self.included(base)
      base.send :alias_method, :migrate, :migrate_with_octopus
      base.send :alias_method, :migrations, :migrations_with_octopus
    end

    def migrate_with_octopus(&block)
      return migrate_without_octopus(&block) unless connection.is_a?(Octopus::Proxy)
      migrations(true).map(&:shards).flatten.map(&:to_s).uniq.each do |shard|
        connection.run_queries_on_shard(shard) do
          ActiveRecord::SchemaMigration.create_table
          ActiveRecord::InternalMetadata.create_table
          migrate_without_octopus(&block)
        end
      end
    rescue ActiveRecord::UnknownMigrationVersionError => e
      raise unless migrations(true).detect { |m| m.version == e.version }
    end

    def migrations_with_octopus(shard_agnostic = false)
      migrations = migrations_without_octopus
      return migrations if !connection.is_a?(Octopus::Proxy) || shard_agnostic

      migrations.select { |m| m.shards.include?(connection.current_shard.to_sym) }
    end
  end
end

ActiveRecord::Migrator.send(:include, Octopus::Migrator)

[kevinjcoleman]

@PhilCoggins feel free to make a PR, I don't have the time at the moment and @thiagopradi isn't really maintaining the gem anymore. If you make a PR though he might merge it!

[PhilCoggins]

I'll make a PR and take lead, but would like to hear from @thiagopradi on my snippet above first as I'm sure he can better explain some of the complexities here. I have also noticed rollbacks have some issues on selecting the correct shard.

I know he's not maintaining the gem anymore, but it's my understanding that he is committed to helping to provide a clean migration path until we get to Rails 6.

[waynehoover]

I think this is what is causing my error: #512 Though I am not sure because I am running a 4.2 app.

In the meantime, I'm not sure where to put your monkey patch @PhilCoggins as in an initializer I get: NameError: undefined local variable or method `connection' for #<ActiveRecord::Migrator:0x00007ff9649aa7f8>

I have taken to just forking the gem and removing the migration support all together as I don't need it.

[ajaleelp]

@PhilCoggins We just faced the exact issue you described above and your monkey-patch helped! (Thank you)
I was wondering if you ever got to work with that PR? 😃

[PhilCoggins]

@ajaleelp glad that it helped, I never worked on a fix as I don't feel comfortable with the code base, and I never heard back from the author when I pinged for help several times. Just working my way to Rails 6...