Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrations from other ORMs cause mongo migrations to error. #59

Open
jgandt-wp opened this issue Aug 23, 2022 · 1 comment · May be fixed by #61
Open

Migrations from other ORMs cause mongo migrations to error. #59

jgandt-wp opened this issue Aug 23, 2022 · 1 comment · May be fixed by #61

Comments

@jgandt-wp
Copy link

jgandt-wp commented Aug 23, 2022

Given:

  • running ActiveRecord migrations (provided by rails) alongside Mongo migrations (provided by this gem) within a single rails project.
  • version == 202201010101 is a valid Active Record migration in your project.
  • version == 202201010101 is NOT avalid Mongo migration in your project

Consider:

rake db:migrate VERSION=202201010101

Expected:

  • The command should migrate the ActiveRecord data store to the 202201010101 version.
  • The command should complete successfully with Mongo migrations being a no-op.

Actual:

  • The command migrates the ActiveRecord data store to the 202201010101 version.
  • The command errors because mongoid_rails_migrations is unable to find the 202201010101 version to migrate to.

My current workaround is a monkeypatch hack of Mongoid::Migrator.migrate to gracefully exit from mongo migrations. It's an ORM specific (ActiveRecord) solution. Ultimately I think the correct course of action is completely divorcing mongoid_rails_migrations from db:migrate and other default ActiveRecord rake tasks. However this is breaking behavior.

I'm happy to code up solutions to this if you have any other suggestions on reasonable/generic ways to solve this.

My monkey patch:

module Mongoid
  class Migrator
    class << self
      # override from https://github.com/adacosta/mongoid_rails_migrations/blob/2b8b1c98bac36d0332f7408c0ca2d8a9da308e37/lib/mongoid_rails_migrations/active_record_ext/migrations.rb#L205
      # This allows us to handle non-Mongo migration versions gracefully
      alias :old_migrate :migrate
      def migrate(migrations_path, target_version = nil)
        target_mongo_version_exists = Mongoid::Migrator.get_all_versions.index(target_version.to_i)
        target_rails_version_exists = ActiveRecord::Base.connection.migration_context.migrations.index { |migration| migration.version == target_version.to_i }

        if target_mongo_version_exists || (target_rails_version_exists.nil? && target_mongo_version_exists.nil?)
          old_migrate(migrations_path, target_version)
        elsif target_rails_version_exists
          # There is a valid migration here for Rails so just exit
          return
        else
          # we shouldn't actually ever hit this case as the rails migration rake task should error out before we get here
          # this is included for completeness and safety
          raise Mongoid::UnknownMigrationVersionError.new(target_version)
        end
      end
    end
  end
end

Note that this patch does not allow you to run mongoid migrations via db:migrate VERSION=. It simply allows you to run rails migrations without erroring. I wrote up separate rake tasks for mongoid migrate tasks targeting a version. I really didn't feel like patching ActiveRecord just to get this to work.

@jarthod
Copy link
Collaborator

jarthod commented Aug 24, 2022

Hi,

Indeed this gem wasn't really made to support the case of using AR + Mongoid at the same time. But as it is adding to the db:migrate task, it makes sense to play nice with other ORMs, even if it's a bit "hard-coded". I think a patch like this (to avoid raising an error if the version matches an AR version) is a good addition as long as it only applies if ActiveRecord is in use (I don't know if that's easy to do? can we check if the rake task is loaded or not?)

@heiytor heiytor linked a pull request Dec 4, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants