Skip to content
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

Added an analogue ActiveRecord adapter for Sequel #380

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AlexeyMatskevich
Copy link

Hi, this is my attempt to add an adapter for Sequel with functionality similar to ActiveRecord.
From the disadvantages of the adapter I will note several items:

  1. No support for aliases, I haven't found analog of attribute_alias for Sequel
  2. I don't fully understand what #associate_all and #associate does for ActiveRecord adapter, maybe my implementation doesn't quite fit as an analogue.
  3. Kaminari doesn't support Sequel, for that reason I used an implementation from the Sequel extension that doesn't support analog of padding.
  4. Perhaps Sequel #disconnect does not do the same as ActiveRecord #clear_active_connections!

@richmolj
Copy link
Contributor

Hey @AlexeyMatskevich sorry for the delay, I've had 👀 on this for a while but haven't commented. Great work. Very, very cool to see ❤️

Re: associate/associate_all - basically only there because AR will hit the DB as a side effect when using plain accessor logic, you might now need anything here.

Past that, I think this probably makes the most sense as a separate gem. I'd be happy to put it in the graphiti-api org and/or link to it on the documentation website, though.

Specs are the last thing. It'd be nice to have a kind of "adapter spec suite" we could run to validate an adapter works. Too bad we don't have that right now, but some sort of testing would be good.

Again, this was great to see!

@AlexeyMatskevich
Copy link
Author

Hey @richmolj I could try writing specifications for an ActiveRecord adapter to use this test set afterwards to compare the capabilities of the adapter for Seqeul with the standard adapter (ActiveRecord).
Then wee can take a look at the overall code and put the tests for AR in a separate pull request and the code of adapter for Sequel with tests in a separate gem.

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

Successfully merging this pull request may close these issues.

3 participants