evilmartians / evil-seed

A Gem for creating partial anonymized dumps of your database using your app model relations.
MIT License
447 stars 19 forks source link

[Need to discuss] Fix ActiveRecord 4.2 support with backport of Relation#in_batches #3

Closed Envek closed 7 years ago

Envek commented 7 years ago

Well, gem doesn't work with Rails 4.2 :bomb:

If I run locally

appraisal activerecord-4-2 rake test

it fails with absolutely clear message:

NoMethodError: undefined method `in_batches' for #<Forum::ActiveRecord_Relation:0x007fe48f461650>

But (and that is weird) Travis builds passes just fine :trollface: See build for today master: https://travis-ci.org/evilmartians/evil-seed/builds/232026609)

The ActiveRecord::Relation#in_batches method allows to access already batched relation objects.

For us it allows to fetch record attributes without instantiating AR model objects which is slow and heavy.

Unfortunately it was introduced only in ActiveRecord 5.0.

Solution that I've tried here is to backport this method with dependencies from current Rails master and to prevent breakage of anything to backport with refinements (so it is only available inside RelationDumper class).

I want to discuss, how much this solution is good, bad, and ugly?

sponomarev commented 7 years ago

I like the approach. The only problem that I'm worried about is that we need a failing test for Rails 4.2 to make the purpose of this change obvious.

palkan commented 7 years ago

Yep, refinements are great)

And agree with @sponomarev about specs.

According to Travis success – https://travis-ci.org/evilmartians/evil-seed/jobs/232026613 ; it was using Rails 5. I've fixed it in master (and now it's failing).

Envek commented 7 years ago

Have no idea what failing test to write, when all existing tests fails on 4.2 (so they will fail again if refinement will be removed), so that should be enough IMO.

Rebased this branch on top of current master (with successfully failing Travis). Now Travis should successfully pass.

sponomarev commented 7 years ago

Yeah, since the problem was in the matrix itself, we have enough coverage for the issue 😄

palkan commented 7 years ago

@Envek Mysql specs failed for 4.2 😞

Envek commented 7 years ago

To execute multi-statement queries in mysql2 connections the MULTI_STATEMENTS client flag must be set. But ability to pass flags to mysql2 adapter is available only since AR 5.0: https://github.com/rails/rails/commit/46fe546c5f8bf505eb2503e0a784991c69a1d223

Tonight will try to construct custom mysql2 connection for testing database restoration on 4.2.

palkan commented 7 years ago

I think, we can just say "Sorry, it's not working for mysql+Rails4; use Rails5 or propose a PR". Let's remove this case from the build matrix and merge this.

Envek commented 7 years ago

Excluded. Gem itself is working fine (it creates dumps), it just can't be tested easily against mysql+AR4.

andyatkinson commented 5 years ago

Can you clarify whether this is supposed to work with this setup: Rails 4.2.11, Jruby 9.2.5.0, Postgres ('activerecord-jdbcpostgresql-adapter'). I'm seeing the same break with in_batches not being available on active record relation, but it also looks like Rails 4.2 is supported, so I'm confused. Thanks.

palkan commented 5 years ago

Probably, this is caused by JRuby+Refinements bug (https://github.com/jruby/jruby/issues/5054). Quick fix is to add using self to the Refinement module. @andyatkinson could you try it?

andyatkinson commented 5 years ago

@palkan I tried adding using self in my local gem copy the line after the end of the refine ... do end block, so I think this is saying using with this module name, which seems to be the same as what is happening here in relation dumper: https://github.com/evilmartians/evil-seed/blob/master/lib/evil_seed/relation_dumper.rb#L7.

Getting the same error.

NoMethodError: undefined method `in_batches' for #<ActiveRecord::Relation []>
Did you mean?  find_in_batches
$ ruby -v
jruby 9.2.5.0 (2.5.0) 2018-12-06 6d5a228 Java HotSpot(TM) 64-Bit Server VM 25.144-b01 on 1.8.0_144-b01 +jit [darwin-x86_64]

So to me it seems like the refine block isn't being picked up properly. If I have some more time, I'll try the simple example you did here https://github.com/jruby/jruby/issues/5054 with jruby 9.2.5.0 locally. If you are able to try it out, let me know. We're hoping to use evil seed, it looks cool! We're probably going to be on Rails 4.2 for a bit longer. :)

andyatkinson commented 5 years ago

Putting that code into a file and running ruby deep_dup.rb, with the using self after the refine block, it prints All OK. 🤔

Envek commented 5 years ago

@palkan, the workaround with using self or similar doesn't help. It seems to be some different bug in JRuby (perhaps related) because there is no calling of refined methods from the refine block.

Here method in_batches doesn't appear in ActiveRecord::Relation altogether after using EvilSeed::Refinements::InBatches was invoked.

I've opened https://github.com/evilmartians/evil-seed/pull/8 with all infrastructure prepared for adding JRuby (Travis matrix and Gemfile adjustments), please play with it if you will have a spare minute.