Skip to content
This repository

Use OrmAdapter instead of talking directly to ActiveRecord #105

Closed
ryana opened this Issue October 03, 2010 · 51 comments
Ryan Angilly

Hi,

I just tried to play with this in an app with no ActiveRecord. I was disappointed. It seems the only reason the engine relies on AR is to provide History functionality. I would argue that having the History in a database, and therefore tying the app to AR & SQL, isn't worth it. How about we change it to just dump to a CSV and remove the AR dep?

$0.02

jack dempsey

I don't think I'd like it in a CSV for a variety of reasons, but I do agree that it doesn't have to be AR. I would imagine if there was a patch and work done towards allowing a pluggable History model, it'd be at least considered. I don't have the time at the moment to do this, but I'd imagine you could start with abstracting out the calls to read/write History, ultimately allowing for a drop in of any storage structure.

In fact, it might be interesting to leverage wycats moneta towards this end: http://github.com/wycats/moneta

Erik Michaels-Ober
Owner

I'd like to port the code from being AR-specific to using OrmAdapter. It's a new project, but it has promise: http://github.com/ianwhite/orm_adapter/

I have renamed this issue accordingly.

Ryan Angilly

Out of curiosity, do people actually use the History. Seems like the kind of feature I could see myself adding into a project because "it seems cool" and then never actually using.

Erik Michaels-Ober
Owner

When working in an organization with multiple people, the history feature provides an audit log of who created/edited/updated which records and when. IMHO, that is an important use case. That said, I would accept a patch to allow disabling of history via the configuration DSL.

Am I wrong in believing that using OrmAdapter instead of calling ActiveRecord directly would solve this problem for you?

Ryan Angilly

You are not wrong in believing that. I've never used rails_admin, or paid attention to the mailing list. I was innocently just wondering if anybody even used the feature. Does it ever get discussed, do people file bugs on it, do people make feature requests on it? Just wondering.

tl;dr: You're right. I wasn't trying to be a dick, just being curious :)

Fredrik Björk

+1 to making rails_admin ORM agnostic. Has any work started on this already?

Erik Michaels-Ober
Owner

See #157 for my thoughts on supporting multiple ORMs.

It's definitely something I'd like to see. I don't think anybody is working on it yet. Would you have any interest in adding support for more ORMs at the RailsAdmin BugMash on Saturday?

Ramon Brooker

I'll be testing it on my fork and generating a patch that could be added to the trunk, using the orm, like devise did... in terms of the history, perhaps adding an option to --skip-history would be a solution to that issue, I can see situations were it wouldn't be needed if there were only one administrator and if memory were an issue, I'll see if can add that too.
Cheers

Maciej Lotkowski
ml commented January 10, 2011

Is anybody working on this one ATM? If not, I think I can try.

Erik Michaels-Ober
Owner

Go for it. I justed added your name to the ticket. Please post updates here as you go.

Steve Klabnik

I'm going to leave a comment here along with my +1, so that I can get an update if this happens. I don't have time to build it myself, but I'd love to use it...

Maciej Lotkowski
ml commented January 11, 2011

It turns out, that orm_adapter lacks some important methods like count and equivalent of ActiveRecord::Base#table_exists? I need to tinker with it for a while before continuing.

Erik Michaels-Ober
Owner

You might want to take a look at veneer as an alternative: https://github.com/hassox/veneer

It looks like Daniel is now maintaining it again.

Maciej Lotkowski
ml commented January 12, 2011

Looks promising.

Marius

Guys,

I am working on (quick and dirty) port to mongoid. I have both AR (mysql) and mongoid in my local app.

https://github.com/yaptv/rails_admin/tree/mongoid

Models are properly recognized, listed and displayed for Mongoid, in my local setup. No relationships for the time being.

Ryan Bates

How about this, turn off the history feature by default, and only enable it if there's a class called History (or maybe something more specific). You can provide a generator for creating this class.

rails g rails_admin:history

The default behavior would be to generate an ActiveRecord model with a database back-end, but one can customize this to use whatever ORM and back-end they prefer.

Erik Michaels-Ober
Owner

That sounds reasonable to me.

Lucas Stephanou

after some months busy, I'ill take a look on that in next days, using ryan approuches.
Some questions:

  • Rely on ActiveModel(in someway) isn't a good solution?
  • Use mixins(include) is much magic/ugly, or we can think of using it( do some introspection and use drivers for AR, Mongo, etc... so any yet more new ORM can be supported in future)
Dan Kubb
dkubb commented March 26, 2011

I did a quick and dirty code spike to see what it would take to integrate DataMapper and Rails Admin: https://bit.ly/gc10ff

I'm still doing some local testing, but it looks like it's mostly functional now. Once I'm done, I'd be happy to work with others to see if orm_adapter (or veneer? others?) provides the interfaces we need, and then doing whatever is needed for implementing the DataMapper side of things. It would be awesome to collaborate with users/authors of other ORMs.

Lucas Stephanou

dkubb,
seems nice.
About my last comment, I did great findings with introspection.
You did want to work more with multi ORM support?

Dan Kubb
dkubb commented March 28, 2011

@lucasts: I don't know if I'd rely on ActiveModel compatibility, since you're exclude Sequel, DataMapper and other ORMs. Besides, AMo only really provides an interface for some parts of an ORM, and afaik it doesn't have anything to say about the querying/updating side, which is what we need.

A mixin could work, although if the mixin needs to define a method that is already used by the ORM then you could break normal functionality of the class, especially if other methods depend on the methods you replaced.

IMHO the best approach is using a proxy that wraps the objects and provides a uniform interface that RailsAdmin uses. The underlying object doesn't need to know anything about RailsAdmin, and you're not monkey patching the classes, so there is no chance of conflict. I haven't yet looked into orm_adapter or veneer, but I would hope they take this approach.

Lucas Stephanou

You're right about mixin(aka monkey patching).
It will became a stopper for RailsAdmin.

I didn't thought about using proxy pattern to approach this, but seems a great solution and even a elegant solution.

We can rely on RailsAdmin instropection, find which ORM that class is using and RailsAdmin use it, w/o knowing if it's a DataMapper objetct or AR.

This can be done with a little ProxyFactory. And that way you can use RA with multiple ORMs in the same project.
To solve History class problem, we can create a new configuration option, asking wich orm the user want to History class use.

Ryan Bates
ryanb commented March 28, 2011

Speaking of the History feature. Maybe consider moving this into a separate gem entirely (rails_admin_history)? It would be nice if Rails Admin itself is modular enough so extensions like this could be added as separate gems and developed by others.

Erik Michaels-Ober
Owner

@ryanb: Agreed. The next major project for RasilAdmin is to create an extensions API for things like history. This way there could be a separate history gem for people using MongoDB or whatever.

Maciej Lotkowski
ml commented April 07, 2011

I worked (https://github.com/ml/veneer/tree/properties) on property and constraints reflections for Veneer in order to integrate it with rails_admin with @kaapa. Properties is pretty much done while constraints/validations still needs some work. I won't have time to work on this in next couple of months. If anybody is interested, I can clean things up and push what I have to github.

Petteri Kääpä
Collaborator
kaapa commented April 08, 2011

@ml: Thank's for all the work you've done so far, it's been solid stuff!

I'd be interested to (try) pick up where you left after I return home from holiday. I'll be back on 18th so if you could clean and push by around then it would be cool.

Btw. are you reachable via e-mail during the later half of month if I have questions about something?

Maciej Lotkowski
ml commented April 10, 2011

@kaapa: Sure, I'll be glad to clarify stuff.

Paul Strong

Oh yes, being able to run this on project that uses mongoid would be awesome. If the history feature is in the way of porting to orm_adapter/veneer then please refactor it into a separate gem. Although nice, I do not use this feature and I suspect that the same applies to many others.

I don't think this should be too much work. In a couple of hours on my fork I was able to mostly port it to Mongoid, but I wiped ActiveRecord out in the process so it's definitely just a temporary solution for us. If someone could jumpstart this on a branch I'd be happy to try and contribute a bit.

Perhaps the first step should be to refactor the history feature out into its own gem. This would clear the way for porting to orm_adapter/veneer.

@ml: Have you worked with orm_adapter? How does it compare to veneer?

Adam McDonald
raid5 commented May 01, 2011

+1 for Mongoid support!

Maciej Lotkowski
ml commented May 04, 2011

@lapluviosilla I haven't worked with orm_adapter. I just prefer Veneer's implementation over orm_adapter's and that why I chose it.

Siong
Petteri Kääpä
Collaborator
kaapa commented May 04, 2011

@siong1987: I started working with Veneer integration yesterday. I'll push code to a topic branch when it gets to such state if folks want to chip in or follow the progress. I'll write bit more once I get the initial exploration & planning out of the way.

Siong
Ben Zittlau

I would love to see this working on Mongoid.

Eugene Chen
ideaOwl commented May 11, 2011

It would really be nice if it works on Mongoid.

Petteri Kääpä
Collaborator
kaapa commented May 11, 2011

For Mongoid support, someone would need to write a Veneer adapter for Mongoid. Veneer provides MongoMapper adapter which might make it an easy port. Anyone?

By the way Veneer integration is slowly, but steadily progressing. There's a ton of missing features in my current branch, but I believe it's still coming together at solid pace. I'll make the branch public soon as I get little bit more explorative coding out of the way...

Benoit Bénézech
Collaborator

I don't know if it's the right move...
Can't we 'just' write an adapter for mongoid, which would cover 95% of demand? I feel like generic adapter is a very bad idea here.
I'd rather use the API already defined by ActiveRecord adapter inside RA, which is quite :cool: btw, and write an adapter for Mongoid, then maye a third one for Datamapper and thus cover nearly all needs.
Using Veneer won't help us anyway, since what already exists for Datamapper/Mongomapper does not not go nearly far enough.
Code would also be less scattered, easier to read inside RailsAdmin. Plus some work has already be done for Mongoid/DataMapper, and ActiveRecord is already implemented!

Petteri Kääpä
Collaborator
kaapa commented June 09, 2011

@bbenezech, I think I have to agree with you, although I initially hoped we could get a 3rd party abstraction layer. Perhaps we should move on as you suggest as I'm afraid I don't have the resources to push the Veneer integration forward in the very near future...

PS. I'm at work right now so I'll write a longer description of the Veneer integration pain points when back home.

Thijs Cadier
thijsc commented June 16, 2011

We'd also love to be able to use a Mongoid version of Rails Admin. We'd be willing to invest time in it as well, does anybody have any idea on when there will be consensus on the path to take?

Benoit Bénézech
Collaborator

Kaapa can't work on Veneer and we agree using an adapter may be a better idea.
If you can write an adapter for Mongoid, I'm 100% behind you and I'll help as much as I can. At some point it'd be good to have a Mongoid commiter invest some time on it too.
Someone already worked on it some time ago, check the forks.
First we'll need to find a way to make RailsAdmin universe work with two adapters (dummy_app, specs, CI, model detection, etc.)

Siong

If we agree on doing adapter for mongoid, this fork seems like a good start: https://github.com/yaptv/rails_admin/tree/mongoid

Yorick Holkamp

Or one updated more recently: https://github.com/fjg/rails_admin

Cyprian Kowalczyk

Any updates on Mongoid support progress?

Jamal Mohammed

Any updates on Mongoid support?

Alexey Poimtsev

Any news, guys?

Woody Peterson

Looks like a lot of recent work has gone into abstraction:

https://github.com/sferik/rails_admin/blob/master/lib/rails_admin/adapters/active_record.rb

Neat.

Mitsuhiro Shibuya
Collaborator

I'm currently working on Mongoid support.
https://github.com/mshibuya/rails_admin/tree/mongoid

I'm new to Mongoid, so any suggestions or advices are welcomed.
I'm planning to send a pull request when I've got this more stable and added some more tests.

Benoit Bénézech
Collaborator

hey @mshibuya,

This is awesome. I just had a look at your code, it looks perfect. You even added specs for ActiveRecord adapter (I wasn't finished, thanks for tackling it).

Meanwhile I just made some uncommitted changes to the active_record adapter.

I suggest you merge upstream when I'm done (give me 1 hour), then I'll stop modifying all related code so I can merge this easily when you feel ready.

Thanks again.

Benoit Bénézech
Collaborator

Ok, I just pushed. I mainly cleaned up filters code which was utterly unreadable.

Timo Sand

Hey @mshibuya,

How stable is your fork ATM, does it work enough so I could use it in my app which I'm currently developing?

Mitsuhiro Shibuya
Collaborator

@bbenezech
I see. I'll pull your works on master.
Thanks for checking.

@deiga
I don't think it has enough quality for actual operation.
But if you could give a try and send me a feedback, I would be able to improve it even faster :)

Benoit Bénézech bbenezech closed this March 12, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.