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

ActiveRecord 4.1 support #307

Closed
chewi opened this Issue Feb 4, 2014 · 23 comments

Comments

Projects
None yet
@chewi

chewi commented Feb 4, 2014

Beta1 has been out a while so I thought I'd give it a try. After scratching my head for a while, I realised that I needed to create a squeel/adapters/active_record/4.1 directory but that wasn't enough. The preloading stuff has changed quite a lot. In particular, rails/rails@6e5a2cb removed the run method that we have been applying alias_method_chain to. I'm afraid I am quite lost at this point.

@saki7

This comment has been minimized.

Show comment
Hide comment
@saki7

saki7 Feb 12, 2014

+1, I tried to inspect it but I stuck at the same line.

saki7 commented Feb 12, 2014

+1, I tried to inspect it but I stuck at the same line.

@trestrantham

This comment has been minimized.

Show comment
Hide comment
@trestrantham

trestrantham Feb 17, 2014

I think was able to chain onto ActiveRecord::Associations::Preloader::Association rather than ActiveRecord::Associations::Preloader to get around the removal of run in the rails change above. However, now I'm stuck on an issue where Squeel::Adapters::ActiveRecord::JoinDependencyExtensions is passing an invalid argument via build_with_squeel (https://github.com/activerecord-hackery/squeel/blob/master/lib/squeel/adapters/active_record/join_dependency_extensions.rb#L29) to polyamourous (https://github.com/activerecord-hackery/polyamorous/blob/master/lib/polyamorous/activerecord_4.1/join_dependency.rb#L18). Removing the erroneous parameter seems to ignore squeel but I think that is probably another issue. I ran out of time but I wanted to report my findings here.

trestrantham commented Feb 17, 2014

I think was able to chain onto ActiveRecord::Associations::Preloader::Association rather than ActiveRecord::Associations::Preloader to get around the removal of run in the rails change above. However, now I'm stuck on an issue where Squeel::Adapters::ActiveRecord::JoinDependencyExtensions is passing an invalid argument via build_with_squeel (https://github.com/activerecord-hackery/squeel/blob/master/lib/squeel/adapters/active_record/join_dependency_extensions.rb#L29) to polyamourous (https://github.com/activerecord-hackery/polyamorous/blob/master/lib/polyamorous/activerecord_4.1/join_dependency.rb#L18). Removing the erroneous parameter seems to ignore squeel but I think that is probably another issue. I ran out of time but I wanted to report my findings here.

@spiffistan

This comment has been minimized.

Show comment
Hide comment
@spiffistan

spiffistan Feb 21, 2014

+1, support would be great

spiffistan commented Feb 21, 2014

+1, support would be great

@randomor

This comment has been minimized.

Show comment
Hide comment
@randomor

randomor Feb 21, 2014

Having the same issue with activerecord-hackery/polyamorous#9, the build_with_polymorphism is taking 2 arguments instead of 3 in activerecord_4.1.

randomor commented Feb 21, 2014

Having the same issue with activerecord-hackery/polyamorous#9, the build_with_polymorphism is taking 2 arguments instead of 3 in activerecord_4.1.

@ernie

This comment has been minimized.

Show comment
Hide comment
@ernie

ernie Mar 10, 2014

Member

+1

Member

ernie commented Mar 10, 2014

+1

@vipulnsward

This comment has been minimized.

Show comment
Hide comment
@vipulnsward

vipulnsward Mar 10, 2014

@ernie :)
I have been lazy for the past month, I had tried to bring AR 4.1 compat support, and had started to push some of the work at https://github.com/vipulnsward/squeel/tree/ar-master-compat
I hope to complete this sometime soon.

vipulnsward commented Mar 10, 2014

@ernie :)
I have been lazy for the past month, I had tried to bring AR 4.1 compat support, and had started to push some of the work at https://github.com/vipulnsward/squeel/tree/ar-master-compat
I hope to complete this sometime soon.

@pablocrivella

This comment has been minimized.

Show comment
Hide comment

pablocrivella commented Apr 9, 2014

+1

@hasghari

This comment has been minimized.

Show comment
Hide comment
@hasghari

hasghari commented Apr 13, 2014

👍

@ben-moxon

This comment has been minimized.

Show comment
Hide comment
@ben-moxon

ben-moxon Apr 22, 2014

I would quite like to get this working and having taken a look at the above link, the first thing I'm running into is that in the Preloader Extensions we have lost Preloader.run, so that method chain won't run any more. Apparently it's all "internal to rails" and "low level" so we shouldn't be using it anyway, but if anyone has any suggestions about what it is being used for I might be able to put together a fix.

ben-moxon commented Apr 22, 2014

I would quite like to get this working and having taken a look at the above link, the first thing I'm running into is that in the Preloader Extensions we have lost Preloader.run, so that method chain won't run any more. Apparently it's all "internal to rails" and "low level" so we shouldn't be using it anyway, but if anyone has any suggestions about what it is being used for I might be able to put together a fix.

@chewi

This comment has been minimized.

Show comment
Hide comment
@chewi

chewi Apr 22, 2014

Unfortunately the nature of Squeel requires it to be quite tightly integrated with Rails, which can mean that we have no choice but to use those internal things that we shouldn't be using. Fixing compatibility with 4.1 is an interesting challenge that I wouldn't mind having a shot at but it's a fairly long way down my priority list.

chewi commented Apr 22, 2014

Unfortunately the nature of Squeel requires it to be quite tightly integrated with Rails, which can mean that we have no choice but to use those internal things that we shouldn't be using. Fixing compatibility with 4.1 is an interesting challenge that I wouldn't mind having a shot at but it's a fairly long way down my priority list.

@kakubei

This comment has been minimized.

Show comment
Hide comment
@kakubei

kakubei May 8, 2014

I'm sorry but what's the solution to this then? I'm getting the same error with Polyamorous from the activerecord-hackery repo and ActiveRecord 4.1.

Is it either stay on AR 4.0.4 or remove all traces of Squeel from our app?

kakubei commented May 8, 2014

I'm sorry but what's the solution to this then? I'm getting the same error with Polyamorous from the activerecord-hackery repo and ActiveRecord 4.1.

Is it either stay on AR 4.0.4 or remove all traces of Squeel from our app?

@gamov

This comment has been minimized.

Show comment
Hide comment
@gamov

gamov May 8, 2014

@kakubei Ah! Sounds familiar. I'm stuck on Rails 3.0.x because of meta_where dependency. I'm in the mist of converting all meta_where code to pure arel.

gamov commented May 8, 2014

@kakubei Ah! Sounds familiar. I'm stuck on Rails 3.0.x because of meta_where dependency. I'm in the mist of converting all meta_where code to pure arel.

@chewi

This comment has been minimized.

Show comment
Hide comment
@chewi

chewi May 8, 2014

@kakubei Yes. Or fix it. =P

chewi commented May 8, 2014

@kakubei Yes. Or fix it. =P

@kakubei

This comment has been minimized.

Show comment
Hide comment
@kakubei

kakubei May 8, 2014

@chewi If I could fix it, believe me I would but that's beyond my current knowledge.

kakubei commented May 8, 2014

@chewi If I could fix it, believe me I would but that's beyond my current knowledge.

@ressu

This comment has been minimized.

Show comment
Hide comment
@ressu

ressu May 9, 2014

I ended up ripping out squeel completely. The query interface in Rails has matured quite a bit and the more complex queries I had were quite easy to rewrite to use arel or regular queries.

The biggest advantage of squeel, which is the fact that you can easily refer to columns in different tables turned out to promote misplaced code (queries in wrong models etc), so rewriting helped in spotting those problems.

Squeel definitely has its uses. But for someone like me, who uses it just for the convenience, it just guides you to use wrong coding practices.

ressu commented May 9, 2014

I ended up ripping out squeel completely. The query interface in Rails has matured quite a bit and the more complex queries I had were quite easy to rewrite to use arel or regular queries.

The biggest advantage of squeel, which is the fact that you can easily refer to columns in different tables turned out to promote misplaced code (queries in wrong models etc), so rewriting helped in spotting those problems.

Squeel definitely has its uses. But for someone like me, who uses it just for the convenience, it just guides you to use wrong coding practices.

@kakubei

This comment has been minimized.

Show comment
Hide comment
@kakubei

kakubei May 9, 2014

Well, I loved squeel. It allowed me to write much more elegant and readable queries, but now I'm in the process of removing it from all my code since I can't upgrade AR. It does give you pause when you rely on gems which you don't know if they'll be around next week.

kakubei commented May 9, 2014

Well, I loved squeel. It allowed me to write much more elegant and readable queries, but now I'm in the process of removing it from all my code since I can't upgrade AR. It does give you pause when you rely on gems which you don't know if they'll be around next week.

@gamov

This comment has been minimized.

Show comment
Hide comment
@gamov

gamov May 9, 2014

Have a look here, it might be the perfect balance between removing the 'magic' of squeel and keep the arel verbosity down: https://github.com/camertron/arel-helpers

gamov commented May 9, 2014

Have a look here, it might be the perfect balance between removing the 'magic' of squeel and keep the arel verbosity down: https://github.com/camertron/arel-helpers

@chewi

This comment has been minimized.

Show comment
Hide comment
@chewi

chewi May 9, 2014

@gamov Thanks! It can't do everything I use Squeel for but it's certainly not a bad compromise.

chewi commented May 9, 2014

@gamov Thanks! It can't do everything I use Squeel for but it's certainly not a bad compromise.

@CyborgMaster

This comment has been minimized.

Show comment
Hide comment
@CyborgMaster

CyborgMaster May 18, 2014

Squeel is central to our product design and I love it!! Does anyone know how hard it would be to upgrade? I want to help!

CyborgMaster commented May 18, 2014

Squeel is central to our product design and I love it!! Does anyone know how hard it would be to upgrade? I want to help!

@nguyenchiencong

This comment has been minimized.

Show comment
Hide comment

nguyenchiencong commented Jun 1, 2014

+1

@Loremaster

This comment has been minimized.

Show comment
Hide comment
@Loremaster

Loremaster Jun 4, 2014

+1, we need someone to fix it!

Loremaster commented Jun 4, 2014

+1, we need someone to fix it!

@bigxiang

This comment has been minimized.

Show comment
Hide comment
@bigxiang

bigxiang Jul 1, 2014

Contributor

I created a fork at https://github.com/bigxiang/squeel and tried to make Squeel compatible with Rails 4.1 and Rails 4.2, but I can't promise all features are well even I had passed all tests under Rails 4.1 and 4.2 ( includes multiple associations maybe works unexpectedly ) . you can try it. make sure using ruby-2-rails-edge branch.

Gemfile:

gem 'polyamorous', git: 'git://github.com/activerecord-hackery/polyamorous.git'
gem 'squeel', git: 'git://github.com/bigxiang/squeel.git', branch: 'ruby-2-rails-edge'

By the way, AR 4+ has improved a lot, I think it's good enough for common cases.

Contributor

bigxiang commented Jul 1, 2014

I created a fork at https://github.com/bigxiang/squeel and tried to make Squeel compatible with Rails 4.1 and Rails 4.2, but I can't promise all features are well even I had passed all tests under Rails 4.1 and 4.2 ( includes multiple associations maybe works unexpectedly ) . you can try it. make sure using ruby-2-rails-edge branch.

Gemfile:

gem 'polyamorous', git: 'git://github.com/activerecord-hackery/polyamorous.git'
gem 'squeel', git: 'git://github.com/bigxiang/squeel.git', branch: 'ruby-2-rails-edge'

By the way, AR 4+ has improved a lot, I think it's good enough for common cases.

@bigxiang

This comment has been minimized.

Show comment
Hide comment
@bigxiang

bigxiang Jul 16, 2014

Contributor

I think it could be closed after the branch was merged, Thank you very one :)

Contributor

bigxiang commented Jul 16, 2014

I think it could be closed after the branch was merged, Thank you very one :)

@bigxiang bigxiang closed this Jul 16, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment