NoMethodError (undefined method `exists?' for DataMigrate::DataSchemaMigration:Class)

[rocket-turtle]

Hi there,

this commit broke our code:
9dda061

We use DataMigrate::DataSchemaMigration.exists? to check if any data_migration allready ran.
It's true if we set up a new system. With the commit above we can use DataMigrate::DataSchemaMigration.instance.exists? for the check. Is it the correct way to do this or should we use a different technique?

Thank you for your help and time providing this gem.

> require('data_migrate/version')
 => true
> DataMigrate::VERSION
 => "8.1.1"
> DataMigrate::DataSchemaMigration.exists?
  DataMigrate::DataSchemaMigration Exists? (0.1ms)  SELECT 1 AS one FROM "data_migrations" LIMIT ?  [["LIMIT", 1]]
 => true
> DataMigrate::DataSchemaMigration.method(:exists?)
 => #<Method: DataMigrate::DataSchemaMigration(version: string).exists?(...) /Users/jonas.schoeler/.rvm/gems/ruby-2.7.6@mhmr/gems/activerecord-6.0.6/lib/active_record/querying.rb:21>

> DataMigrate::VERSION
 => "8.2.0"
> DataMigrate::DataSchemaMigration.exists?
Traceback (most recent call last):
        1: from (irb):1
NoMethodError (undefined method `exists?' for DataMigrate::DataSchemaMigration:Class)
> DataMigrate::DataSchemaMigration.instance.exists?
   Exists? (0.1ms)  SELECT 1 AS one FROM "data_migrations" LIMIT ?  [["LIMIT", 1]]
 => true

[ilyakatz]

@defsprite do you have any recommendations on how to go about this?

[elik-ru]

Same with any ActiveRecords methods not explicitly delegated (all, delete_all etc).

I had to nail down version 8.1.1 to get around this problem.

[defsprite]

@ilyakatz I'm not sure whether DataMigrate::DataSchemaMigration was meant to be a "public" API, but folks seem to be actively using it.

If that is the case, we may want to reconsider the delegation approach (i.e revert the change) and look into restructuring how the gem is loaded in https://github.com/ilyakatz/data-migrate/blob/master/lib/data_migrate.rb in order to avoid eagerly making database connections.

What do you think?

[rocket-turtle]

We changed our code to DataMigrate::DataSchemaMigration.instance.exists?

I hope instance is "public".

Issue can be closed if @elik-ru can do the same "fix".

[rocket-turtle]

DataMigrate::DataSchemaMigration changed back to original implementation.

In this commit DataMigrate::DataSchemaMigration delegate to an anonymous subclass of AR::SchemaMigration
9dda061

In this commit kind of reverts the change:
efc9891

• 8.1: https://github.com/ilyakatz/data-migrate/blob/8.1.1/lib/data_migrate/data_schema_migration.rb
  ** Inherit from AR SchemaMigration directly
• 8.4 / 9.1: https://github.com/ilyakatz/data-migrate/blob/v9.1.1/lib/data_migrate/data_schema_migration.rb
  ** Delegate to anonymous subclass of AR::SchemaMigration
• 9.2: https://github.com/ilyakatz/data-migrate/blob/v9.2.0/lib/data_migrate/data_schema_migration.rb
  ** Inherit from AR SchemaMigration directly

@defsprite, @wildmaples I'm a little bit confused, which way is better? And what were the intentions behind the changes.

[defsprite]

@rocket-turtle The commit was meant as a fix for #222 - because just loading the gem forces an active database connection to be present. However, a lot of (or very vocal) people seem to be relying on the fact that DataMigrate::DataSchemaMigration inherits from AR::SchemaMigration already, so this needs to be done either in a major breaking release or we'd need to refactor the way the gem is loaded instead (i.e. deferring requiring the migrations in a config.to_initialize block or similar).

[rocket-turtle]

Thank you for your reply.

If I understand it correct the ActiveRecord in DataSchemaMigration < ::ActiveRecord::SchemaMigration was the "problem".

So with the switch back to "inherit from ActiveRecord::SchemaMigration" the gem loads AR to early?
Can you confirm, that "your" problem is back?
Do you know why the switch back happend? I only see #275 which reads more like an cleanup not an intentional switch back.

Is there a way to navigate to the pull request of an commit. For example from 9dda061 to #229

[defsprite]

Yes, I can verify this problem exists in version 9.2.0 using the bug app provided. DataSchemaMigration is an AR model and eagerly requiring it in lib/data_migrate.rb forces the connection to be present.

I think there are two ways to go about this:

• Use the delegation approach (which was accidentally removed)
• Fix the eager requiring behaviour by using e.g. the mechanics provided by Rails::Railtie

I'd probably tend to the latter solution, because the delegation approach is confusing and unobvious (as it looks to me it was just accidentally reverted).

The problem described (forcing a db connection to be present) is also a widespread issue in many gems, so you'll end up up using a nulldb adapter approach anyway at some point. We may as well just close the issue #222 as wontfix.

Apologies to the community for creating breaking API changes for a rather minor issue that cause extra work dealing with it (for the second time now)

TLDR: I'd recommend keeping the inheritance approach and fix the gem loading eventually.

[ngan]

FWIW..

Rails 7.1 makes ActiveRecord::SchemaMigration not an actual model (as in, not inherit from ActiveRecord::Base):
https://github.com/rails/rails/blob/main/activerecord/lib/active_record/schema_migration.rb#L8