citusdata / activerecord-multi-tenant

Rails/ActiveRecord support for distributed multi-tenant databases like Postgres+Citus
MIT License
717 stars 97 forks source link

Fix SystemStackError in schema_dumper.rb in combination with strong_migrations #257

Closed Laykou closed 2 days ago

Laykou commented 2 weeks ago

When using https://github.com/ankane/strong_migrations gem, it also overrides the SchemaDumper.

Reference: https://github.com/Laykou/strong_migrations/blob/master/lib/strong_migrations/schema_dumper.rb

strong_migrations in combination with this gem created following issue:

SystemStackError: stack level too deep (SystemStackError)
.../gems/strong_migrations-1.8.0/lib/strong_migrations/schema_dumper.rb:6:in `new'
.../gems/strong_migrations-1.8.0/lib/strong_migrations/schema_dumper.rb:6:in `initialize'
.../gems/activerecord-multi-tenant-2.4.0/lib/activerecord-multi-tenant/migrations.rb:93:in `initialize'
.../gems/strong_migrations-1.8.0/lib/strong_migrations/schema_dumper.rb:6:in `initialize'
.../gems/activerecord-multi-tenant-2.4.0/lib/activerecord-multi-tenant/migrations.rb:93:in `initialize'
.../gems/strong_migrations-1.8.0/lib/strong_migrations/schema_dumper.rb:6:in `initialize'

Module#prepend is recommended approach to be used here instead of monkey patching as advised here https://github.com/ankane/strong_migrations/pull/270#issuecomment-2315160557

Laykou commented 1 week ago

@gokhangulbiz Could this be reviewed and merged, please?

Laykou commented 2 days ago

Closing in favor of #245 245